#60625 - Rendering issue with background and shape overlayed by image

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1782706 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/sl/draw/DrawMasterSheet.java

@@ -33,21 +33,21 @@ public class DrawMasterSheet extends DrawSheet {
     }
 
     /**
-     * Checks if this <code>sheet</code> displays the specified shape.
+     * Checks if this {@code sheet} displays the specified shape.
      *
      * Subclasses can override it and skip certain shapes from drawings,
      * for instance, slide masters and layouts don't display placeholders
      */
     @Override
     protected boolean canDraw(Graphics2D graphics, Shape<?,?> shape) {
+        Slide<?,?> slide = (Slide<?,?>)graphics.getRenderingHint(Drawable.CURRENT_SLIDE);
         if (shape instanceof SimpleShape) {
             // in XSLF, slidenumber and date shapes aren't marked as placeholders opposed to HSLF
             Placeholder ph = ((SimpleShape<?,?>)shape).getPlaceholder();
             if (ph != null) {
-                Slide<?,?> slide = (Slide<?,?>)graphics.getRenderingHint(Drawable.CURRENT_SLIDE);
                 return slide.getDisplayPlaceholder(ph);
             }
         }
-        return true;
+        return slide.getFollowMasterGraphics();
     }
 }

src/java/org/apache/poi/sl/draw/DrawPaint.java

@@ -21,8 +21,6 @@ import java.awt.Color;
 import java.awt.Dimension;
 import java.awt.Graphics2D;
 import java.awt.LinearGradientPaint;
-import java.awt.MultipleGradientPaint.ColorSpaceType;
-import java.awt.MultipleGradientPaint.CycleMethod;
 import java.awt.Paint;
 import java.awt.RadialGradientPaint;
 import java.awt.geom.AffineTransform;
@@ -68,17 +66,27 @@ public class DrawPaint {
                 throw new NullPointerException("Color needs to be specified");
             }
             this.solidColor = new ColorStyle(){
+                    @Override
                     public Color getColor() {
                         return new Color(color.getRed(), color.getGreen(), color.getBlue());
                     }
+                    @Override
                     public int getAlpha() { return (int)Math.round(color.getAlpha()*100000./255.); }
+                    @Override
                     public int getHueOff() { return -1; }
+                    @Override
                     public int getHueMod() { return -1; }
+                    @Override
                     public int getSatOff() { return -1; }
+                    @Override
                     public int getSatMod() { return -1; }
+                    @Override
                     public int getLumOff() { return -1; }
+                    @Override
                     public int getLumMod() { return -1; }
+                    @Override
                     public int getShade() { return -1; }
+                    @Override
                     public int getTint() { return -1; }
                 };
         }
@@ -90,6 +98,7 @@ public class DrawPaint {
             this.solidColor = color;
         }
 
+        @Override
         public ColorStyle getSolidColor() {
             return solidColor;
         }
@@ -133,7 +142,9 @@ public class DrawPaint {
 
     protected Paint getTexturePaint(TexturePaint fill, Graphics2D graphics) {
         InputStream is = fill.getImageData();
-        if (is == null) return null;
+        if (is == null) {
+            return null;
+        }
         assert(graphics != null);
 
         ImageRenderer renderer = DrawPictureShape.getImageRenderer(graphics, fill.getContentType());
@@ -237,8 +248,12 @@ public class DrawPaint {
      *
      */
     private static void applyHslModOff(double hsl[], int hslPart, int mod, int off) {
-        if (mod == -1) mod = 100000;
+        if (mod == -1) {
-        if (off == -1) off = 0;
+            mod = 100000;
+        }
+        if (off == -1) {
+            off = 0;
+        }
         if (!(mod == 100000 && off == 0)) {
             double fOff = off / 1000d;
             double fMod = mod / 100000d;
@@ -253,7 +268,9 @@ public class DrawPaint {
      */
     private static void applyShade(double hsl[], ColorStyle fc) {
         int shade = fc.getShade();
-        if (shade == -1) return;
+        if (shade == -1) {
+            return;
+        }
 
         double fshade = shade / 100000.d;
 
@@ -268,33 +285,39 @@ public class DrawPaint {
      */
     private static void applyTint(double hsl[], ColorStyle fc) {
         int tint = fc.getTint();
-        if (tint == -1) return;
+        if (tint == -1) {
+            return;
+        }
 
         double ftint = tint / 100000.f;
 
         hsl[2] = hsl[2] * ftint + (100 - ftint*100.);
     }
 
-
     protected Paint createLinearGradientPaint(GradientPaint fill, Graphics2D graphics) {
+        // TODO: we need to find the two points for gradient - the problem is, which point at the outline
+        // do you take? My solution would be to apply the gradient rotation to the shape in reverse
+        // and then scan the shape for the largest possible horizontal distance
+        
         double angle = fill.getGradientAngle();
+        if (!fill.isRotatedWithShape()) {
+            angle -= shape.getRotation();
+        }
+
         Rectangle2D anchor = DrawShape.getAnchor(graphics, shape);
+        final double h = anchor.getHeight(), w = anchor.getWidth(), x = anchor.getX(), y = anchor.getY();
 
-        AffineTransform at = AffineTransform.getRotateInstance(
+        AffineTransform at = AffineTransform.getRotateInstance(Math.toRadians(angle), anchor.getCenterX(), anchor.getCenterY());
-            Math.toRadians(angle),
-            anchor.getX() + anchor.getWidth() / 2,
-            anchor.getY() + anchor.getHeight() / 2);
 
-        double diagonal = Math.sqrt(anchor.getHeight() * anchor.getHeight() + anchor.getWidth() * anchor.getWidth());
+        double diagonal = Math.sqrt(h * h + w * w);
-        Point2D p1 = new Point2D.Double(anchor.getX() + anchor.getWidth() / 2 - diagonal / 2,
+        Point2D p1 = new Point2D.Double(x + w / 2 - diagonal / 2, y + h / 2);
-                anchor.getY() + anchor.getHeight() / 2);
         p1 = at.transform(p1, null);
 
-        Point2D p2 = new Point2D.Double(anchor.getX() + anchor.getWidth(), anchor.getY() + anchor.getHeight() / 2);
+        Point2D p2 = new Point2D.Double(x + w, y + h / 2);
         p2 = at.transform(p2, null);
 
-        snapToAnchor(p1, anchor);
+//        snapToAnchor(p1, anchor);
-        snapToAnchor(p2, anchor);
+//        snapToAnchor(p2, anchor);
 
         if (p1.equals(p2)) {
             // gradient paint on the same point throws an exception ... and doesn't make sense
@@ -310,21 +333,7 @@ public class DrawPaint {
             colors[i++] = (fc == null) ? TRANSPARENT : applyColorTransform(fc);
         }
 
-        AffineTransform grAt  = new AffineTransform();
+        return new LinearGradientPaint(p1, p2, fractions, colors);
-        if(fill.isRotatedWithShape()) {
-            double rotation = shape.getRotation();
-            if (rotation != 0.) {
-                double centerX = anchor.getX() + anchor.getWidth() / 2;
-                double centerY = anchor.getY() + anchor.getHeight() / 2;
-
-                grAt.translate(centerX, centerY);
-                grAt.rotate(Math.toRadians(-rotation));
-                grAt.translate(-centerX, -centerY);
-            }
-        }
-
-        return new LinearGradientPaint
-            (p1, p2, fractions, colors, CycleMethod.NO_CYCLE, ColorSpaceType.SRGB, grAt);
     }
 
     protected Paint createRadialGradientPaint(GradientPaint fill, Graphics2D graphics) {
@@ -420,9 +429,13 @@ public class DrawPaint {
     }
 
     private static double HUE2RGB(double p, double q, double h) {
-        if (h < 0d) h += 1d;
+        if (h < 0d) {
+            h += 1d;
+        }
 
-        if (h > 1d) h -= 1d;
+        if (h > 1d) {
+            h -= 1d;
+        }
 
         if (6d * h < 1d) {
             return p + ((q - p) * 6d * h);

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFColor.java

@@ -74,42 +74,52 @@ public class XSLFColor {
 
     public ColorStyle getColorStyle() {
         return new ColorStyle() {
+            @Override
             public Color getColor() {
                 return _color;
             }
 
+            @Override
             public int getAlpha() {
                 return getRawValue("alpha");
             }
 
+            @Override
             public int getHueOff() {
                 return getRawValue("hueOff");
             }
 
+            @Override
             public int getHueMod() {
                 return getRawValue("hueMod");
             }
 
+            @Override
             public int getSatOff() {
                 return getRawValue("satOff");
             }
 
+            @Override
             public int getSatMod() {
                 return getRawValue("satMod");
             }
 
+            @Override
             public int getLumOff() {
                 return getRawValue("lumOff");
             }
 
+            @Override
             public int getLumMod() {
                 return getRawValue("lumMod");
             }
 
+            @Override
             public int getShade() {
                 return getRawValue("shade");
             }
 
+            @Override
             public int getTint() {
                 return getRawValue("tint");
             }
@@ -141,7 +151,9 @@ public class XSLFColor {
                 }
                 // find referenced CTColor in the theme and convert it to java.awt.Color via a recursive call
                 CTColor ctColor = theme.getCTColor(colorRef);
-                if(ctColor != null) color = toColor(ctColor, null);
+                if(ctColor != null) {
+                    color = toColor(ctColor, null);
+                }
             } else if (ch instanceof CTScRgbColor) {
                 // color in percentage is in linear RGB color space, i.e. needs to be gamma corrected for AWT color
                 CTScRgbColor scrgb = (CTScRgbColor)ch;

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShape.java

@@ -58,7 +58,6 @@ import org.openxmlformats.schemas.drawingml.x2006.main.CTSolidColorFillPropertie
 import org.openxmlformats.schemas.drawingml.x2006.main.CTStyleMatrix;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTStyleMatrixReference;
 import org.openxmlformats.schemas.drawingml.x2006.main.STPathShadeType;
-import org.openxmlformats.schemas.drawingml.x2006.main.STSchemeColorVal;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTApplicationNonVisualDrawingProps;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTBackgroundProperties;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTPlaceholder;
@@ -395,9 +394,9 @@ public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
         	// if there's a reference to the placeholder color,
         	// stop evaluating further and let the caller select
         	// the next style inheritance level
-            if (STSchemeColorVal.PH_CLR.equals(solidFill.getSchemeClr().getVal())) {
+//            if (STSchemeColorVal.PH_CLR.equals(solidFill.getSchemeClr().getVal())) {
-                return null;
+//                return null;
-            }
+//            }
             if (phClr == null) {
                 phClr = solidFill.getSchemeClr();
             }
@@ -483,8 +482,7 @@ public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
             }
 
             public boolean isRotatedWithShape() {
-                // TODO: is this correct???
+                return gradFill.getRotWithShape();
-                return (gradFill.isSetRotWithShape() || !gradFill.getRotWithShape());
             }
 
             public GradientType getGradientType() {

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSimpleShape.java

@@ -320,6 +320,12 @@ public abstract class XSLFSimpleShape extends XSLFShape
             public boolean fetch(XSLFShape shape) {
                 CTLineProperties spPr = getLn(shape, false);
                 XSLFFillProperties fp = XSLFPropertiesDelegate.getFillDelegate(spPr);
+
+                if (fp != null && fp.isSetNoFill()) {
+                    setValue(null);
+                    return true;
+                }
+                
                 PackagePart pp = shape.getSheet().getPackagePart();
                 PaintStyle paint = selectPaint(fp, null, pp, theme, hasPlaceholder);
                 if (paint != null) {
@@ -331,39 +337,41 @@ public abstract class XSLFSimpleShape extends XSLFShape
                 if (style != null) {
                     fp = XSLFPropertiesDelegate.getFillDelegate(style.getLnRef());
                     paint = selectPaint(fp, null, pp, theme, hasPlaceholder);
+
+                    // line color was not found, check if it is defined in the theme
+                    if (paint == null) {
+                        paint = getThemePaint(style, pp);
                     }
+                }
+                
                 if (paint != null) {
                     setValue(paint);
                     return true;
                 }
+                
                 return false;
             }
+
+            PaintStyle getThemePaint(CTShapeStyle style, PackagePart pp) {
+                // get a reference to a line style within the style matrix.
+                CTStyleMatrixReference lnRef = style.getLnRef();
+                if (lnRef == null) {
+                    return null;
+                }
+                int idx = (int)lnRef.getIdx();
+                CTSchemeColor phClr = lnRef.getSchemeClr();
+                if(idx <= 0){
+                    return null;
+                }
+
+                CTLineProperties props = theme.getXmlObject().getThemeElements().getFmtScheme().getLnStyleLst().getLnArray(idx - 1);
+                XSLFFillProperties fp = XSLFPropertiesDelegate.getFillDelegate(props);
+                return selectPaint(fp, phClr, pp, theme, hasPlaceholder);
+            }
         };
         fetchShapeProperty(fetcher);
 
-        PaintStyle paint = fetcher.getValue();
+        return fetcher.getValue();
-        if (paint != null) {
-            return paint;
-        }
-
-        // line color was not found, check if it is defined in the theme
-        CTShapeStyle style = getSpStyle();
-        if (style == null) {
-            return null;
-        }
-
-        // get a reference to a line style within the style matrix.
-        CTStyleMatrixReference lnRef = style.getLnRef();
-        int idx = (int)lnRef.getIdx();
-        CTSchemeColor phClr = lnRef.getSchemeClr();
-        if(idx > 0){
-            CTLineProperties props = theme.getXmlObject().getThemeElements().getFmtScheme().getLnStyleLst().getLnArray(idx - 1);
-            XSLFFillProperties fp = XSLFPropertiesDelegate.getFillDelegate(props);
-            PackagePart pp = sheet.getPackagePart();
-            paint = selectPaint(fp, phClr, pp, theme, hasPlaceholder);
-        }
-
-        return paint;
     }
 
     /**

src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFFill.java

@@ -31,6 +31,7 @@ import org.apache.poi.ddf.EscherProperties;
 import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.ddf.EscherSimpleProperty;
 import org.apache.poi.hslf.record.Document;
+import org.apache.poi.hslf.record.RecordTypes;
 import org.apache.poi.sl.draw.DrawPaint;
 import org.apache.poi.sl.usermodel.ColorStyle;
 import org.apache.poi.sl.usermodel.FillStyle;
@@ -38,6 +39,8 @@ import org.apache.poi.sl.usermodel.PaintStyle;
 import org.apache.poi.sl.usermodel.PaintStyle.GradientPaint;
 import org.apache.poi.sl.usermodel.PaintStyle.GradientPaint.GradientType;
 import org.apache.poi.sl.usermodel.PaintStyle.TexturePaint;
+import org.apache.poi.util.BitField;
+import org.apache.poi.util.BitFieldFactory;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -101,6 +104,108 @@ public final class HSLFFill {
      */
     public static final int FILL_BACKGROUND = 9;
 
+    /**
+     * A bit that specifies whether the RecolorFillAsPicture bit is set.
+     * A value of 0x0 specifies that the fRecolorFillAsPicture MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_RECOLOR_FILL_AS_PICTURE = BitFieldFactory.getInstance(0x00400000);
+
+    /**
+     * A bit that specifies whether the UseShapeAnchor bit is set.
+     * A value of 0x0 specifies that the fUseShapeAnchor MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_USE_SHAPE_ANCHOR = BitFieldFactory.getInstance(0x00200000);
+
+    /**
+     * A bit that specifies whether the Filled bit is set.
+     * A value of 0x0 specifies that the Filled MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_FILLED = BitFieldFactory.getInstance(0x00100000);
+
+    /**
+     * A bit that specifies whether the HitTestFill bit is set.
+     * A value of 0x0 specifies that the HitTestFill MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_HIT_TEST_FILL = BitFieldFactory.getInstance(0x00080000);
+    
+    /**
+     * A bit that specifies whether the fillShape bit is set.
+     * A value of 0x0 specifies that the fillShape MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_FILL_SHAPE = BitFieldFactory.getInstance(0x00040000);
+    
+    /**
+     * A bit that specifies whether the fillUseRect bit is set.
+     * A value of 0x0 specifies that the fillUseRect MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_FILL_USE_RECT = BitFieldFactory.getInstance(0x00020000);
+    
+    /**
+     * A bit that specifies whether the fNoFillHitTest bit is set.
+     * A value of 0x0 specifies that the fNoFillHitTest MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_NO_FILL_HIT_TEST = BitFieldFactory.getInstance(0x00010000);
+    
+    /**
+     * A bit that specifies how to recolor a picture fill. If this bit is set to 0x1, the pictureFillCrMod
+     * property of the picture fill is used for recoloring. If this bit is set to 0x0, the fillCrMod property,
+     * as defined in section 2.3.7.6, is used for recoloring.
+     * If UsefRecolorFillAsPicture equals 0x0, this value MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_RECOLOR_FILL_AS_PICTURE = BitFieldFactory.getInstance(0x00000040);
+    
+    /**
+     * A bit that specifies whether the fill is rotated with the shape.
+     * If UseUseShapeAnchor equals 0x0, this value MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_USE_SHAPE_ANCHOR = BitFieldFactory.getInstance(0x00000020);
+    
+    /**
+     * A bit that specifies whether the fill is rendered if the shape is a 2-D shape.
+     * If this bit is set to 0x1, the fill of this shape is rendered based on the properties of the Fill Style
+     * property set. If this bit is set to 0x0, the fill of this shape is not rendered.
+     * If UseFilled is 0x0, this value MUST be ignored. The default value for this property is 0x1.
+     */
+    private static final BitField FILL_FILLED = BitFieldFactory.getInstance(0x00000010);
+    
+    /**
+     * A bit that specifies whether this fill will be hit tested.
+     * If UsefHitTestFill equals 0x0, this value MUST be ignored.
+     * The default value for this property is 0x1.
+     */
+    private static final BitField FILL_HIT_TEST_FILL = BitFieldFactory.getInstance(0x00000008);
+    
+    /**
+     * A bit that specifies how the fill is aligned. If this bit is set to 0x1, the fill is
+     * aligned relative to the shape so that it moves with the shape. If this bit is set to 0x0,
+     * the fill is aligned with the origin of the view. If fUsefillShape equals 0x0, this value MUST be ignored.
+     * The default value for this property is 0x1.
+     */
+    private static final BitField FILL_FILL_SHAPE = BitFieldFactory.getInstance(0x00000004);
+    
+    /**
+     * A bit that specifies whether to use the rectangle specified by the fillRectLeft, fillRectRight,
+     * fillRectTop, and fillRectBottom properties, rather than the bounding rectangle of the shape,
+     * as the filled area. If fUsefillUseRect equals 0x0, this value MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_FILL_USE_RECT = BitFieldFactory.getInstance(0x00000002);
+    
+    /**
+     * A bit that specifies whether this shape will be hit tested as though it were filled.
+     * If UsefNoFillHitTest equals 0x0, this value MUST be ignored.
+     * The default value for this property is 0x0.
+     */
+    private static final BitField FILL_NO_FILL_HIT_TEST = BitFieldFactory.getInstance(0x00000001);
 
 
     /**
@@ -121,6 +226,7 @@ public final class HSLFFill {
 
     public FillStyle getFillStyle() {
         return new FillStyle() {
+            @Override
             public PaintStyle getPaint() {
                 final int fillType = getFillType();
                 // TODO: fix gradient types, this mismatches with the MS-ODRAW definition ...
@@ -150,11 +256,19 @@ public final class HSLFFill {
     
 
     private GradientPaint getGradientPaint(final GradientType gradientType) {
-        final AbstractEscherOptRecord opt = shape.getEscherOptRecord();
+        AbstractEscherOptRecord opt = shape.getEscherOptRecord();
         final EscherArrayProperty ep = HSLFShape.getEscherProperty(opt, EscherProperties.FILL__SHADECOLORS);
         final int colorCnt = (ep == null) ? 0 : ep.getNumberOfElementsInArray();
 
+        // NOFILLHITTEST can be in the normal escher opt record but also in the tertiary record
+        // the extended bit fields seem to be in the second
+        opt = (AbstractEscherOptRecord)shape.getEscherChild(RecordTypes.EscherUserDefined);
+        EscherSimpleProperty p = HSLFShape.getEscherProperty(opt, EscherProperties.FILL__NOFILLHITTEST);
+        int propVal = (p == null) ? 0 : p.getPropertyValue();
+        final boolean rotateWithShape = FILL_USE_USE_SHAPE_ANCHOR.isSet(propVal) && FILL_USE_SHAPE_ANCHOR.isSet(propVal);
+        
         return new GradientPaint() {
+            @Override
             public double getGradientAngle() {
                 // A value of type FixedPoint, as specified in [MS-OSHARED] section 2.2.1.6,
                 // that specifies the angle of the gradient fill. Zero degrees represents a vertical vector from
@@ -162,6 +276,8 @@ public final class HSLFFill {
                 int rot = shape.getEscherProperty(EscherProperties.FILL__ANGLE);
                 return 90-Units.fixedPointToDouble(rot);
             }
+            
+            @Override
             public ColorStyle[] getGradientColors() {
                 ColorStyle cs[];
                 if (colorCnt == 0) {
@@ -179,9 +295,12 @@ public final class HSLFFill {
                 }
                 return cs;
             }
+            
             private ColorStyle wrapColor(Color col) {
                 return (col == null) ? null : DrawPaint.createSolidPaint(col).getSolidColor();
             }
+            
+            @Override
             public float[] getGradientFractions() {
                 float frc[];
                 if (colorCnt == 0) {
@@ -196,9 +315,13 @@ public final class HSLFFill {
                 }
                 return frc;
             }
+            
+            @Override
             public boolean isRotatedWithShape() {
-                return false;
+                return rotateWithShape;
             }
+            
+            @Override
             public GradientType getGradientType() {
                 return gradientType;
             }
@@ -212,14 +335,17 @@ public final class HSLFFill {
         }
 
         return new TexturePaint() {
+            @Override
             public InputStream getImageData() {
                 return new ByteArrayInputStream(pd.getData());
             }
 
+            @Override
             public String getContentType() {
                 return pd.getContentType();
             }
 
+            @Override
             public int getAlpha() {
                 return (int)(shape.getAlpha(EscherProperties.FILL__FILLOPACITY)*100000.0);
             }
@@ -286,11 +412,11 @@ public final class HSLFFill {
     public Color getForegroundColor(){
         AbstractEscherOptRecord opt = shape.getEscherOptRecord();
         EscherSimpleProperty p = HSLFShape.getEscherProperty(opt, EscherProperties.FILL__NOFILLHITTEST);
+        int propVal = (p == null) ? 0 : p.getPropertyValue();
         
-        if(p != null && (p.getPropertyValue() & 0x10) == 0) return null;
+        return (FILL_USE_FILLED.isSet(propVal) && !FILL_FILLED.isSet(propVal))
-
+            ? null
-        return shape.getColor(EscherProperties.FILL__FILLCOLOR, EscherProperties.FILL__FILLOPACITY, -1);
+            : shape.getColor(EscherProperties.FILL__FILLCOLOR, EscherProperties.FILL__FILLOPACITY, -1);
-
     }
 
     /**
@@ -298,22 +424,30 @@ public final class HSLFFill {
      */
     public void setForegroundColor(Color color){
         AbstractEscherOptRecord opt = shape.getEscherOptRecord();
-        if (color == null) {
+        opt.removeEscherProperty(EscherProperties.FILL__FILLOPACITY);
         opt.removeEscherProperty(EscherProperties.FILL__FILLCOLOR);
-            HSLFShape.setEscherProperty(opt, EscherProperties.FILL__NOFILLHITTEST, 0x150000);
+
-        }
+        if (color != null) {
-        else {
             int rgb = new Color(color.getBlue(), color.getGreen(), color.getRed(), 0).getRGB();
             HSLFShape.setEscherProperty(opt, EscherProperties.FILL__FILLCOLOR, rgb);
             int alpha = color.getAlpha();
-            if (alpha == 255) {
+            if (alpha < 255) {
-                opt.removeEscherProperty(EscherProperties.FILL__FILLOPACITY);
-            } else {
                 int alphaFP = Units.doubleToFixedPoint(alpha/255d);
                 HSLFShape.setEscherProperty(opt, EscherProperties.FILL__FILLOPACITY, alphaFP);
             }
-            HSLFShape.setEscherProperty(opt, EscherProperties.FILL__NOFILLHITTEST, 0x150011);
         }
+        
+        EscherSimpleProperty p = HSLFShape.getEscherProperty(opt, EscherProperties.FILL__NOFILLHITTEST);
+        int propVal = (p == null) ? 0 : p.getPropertyValue();
+        propVal = FILL_FILLED.setBoolean(propVal, color != null);
+        propVal = FILL_NO_FILL_HIT_TEST.setBoolean(propVal, color != null);
+        propVal = FILL_USE_FILLED.set(propVal);
+        propVal = FILL_USE_FILL_SHAPE.set(propVal);
+        propVal = FILL_USE_NO_FILL_HIT_TEST.set(propVal);
+        // TODO: check why we always clear this ...
+        propVal = FILL_FILL_SHAPE.clear(propVal);
+
+        HSLFShape.setEscherProperty(opt, EscherProperties.FILL__NOFILLHITTEST, propVal);
     }
 
     /**
@@ -322,10 +456,11 @@ public final class HSLFFill {
     public Color getBackgroundColor(){
         AbstractEscherOptRecord opt = shape.getEscherOptRecord();
         EscherSimpleProperty p = HSLFShape.getEscherProperty(opt, EscherProperties.FILL__NOFILLHITTEST);
+        int propVal = (p == null) ? 0 : p.getPropertyValue();
 
-        if(p != null && (p.getPropertyValue() & 0x10) == 0) return null;
+        return (FILL_USE_FILLED.isSet(propVal) && !FILL_FILLED.isSet(propVal))
-
+            ? null
-        return shape.getColor(EscherProperties.FILL__FILLBACKCOLOR, EscherProperties.FILL__FILLOPACITY, -1);
+            : shape.getColor(EscherProperties.FILL__FILLBACKCOLOR, EscherProperties.FILL__FILLOPACITY, -1);
     }
 
     /**
@@ -349,7 +484,9 @@ public final class HSLFFill {
     public HSLFPictureData getPictureData(){
         AbstractEscherOptRecord opt = shape.getEscherOptRecord();
         EscherSimpleProperty p = HSLFShape.getEscherProperty(opt, EscherProperties.FILL__PATTERNTEXTURE);
-        if (p == null) return null;
+        if (p == null) {
+            return null;
+        }
 
         HSLFSlideShow ppt = shape.getSheet().getSlideShow();
         List<HSLFPictureData> pict = ppt.getPictureData();