<canvas> rectangle fills don't work with gradients

[David Carson]

Mozilla test case fails as the call to fillRect() does not use the specified
fillstyle (in this case a gradient).

Possible solution is implementing FillRect in kjs_html.cpp by building a path,
and using the same Fill code as filling a normal path:

        case Context2D::FillRect: {
            if (args.size() != 4) {
                Object err = Error::create(exec,SyntaxError);
                exec->setException(err);
                return err;
            }
            float x = (float)args[0].toNumber(exec);
            float y = (float)args[1].toNumber(exec);
            float w = (float)args[2].toNumber(exec);
            float h = (float)args[3].toNumber(exec);
            CGContextBeginPath(drawingContext);
            CGContextMoveToPoint (drawingContext, x, y);
            CGContextAddLineToPoint (drawingContext,x+w, y);
            CGContextAddLineToPoint (drawingContext,x+w, y+h);
            CGContextAddLineToPoint (drawingContext,x, y+h);
            CGContextClosePath(drawingContext);

            if (isGradient(contextObject->_fillStyle)) {
                CGContextSaveGState(drawingContext);
                
                // Set the clip from the current path because shading only
                // operates on clippin regions!  Odd, but true.
                CGContextClip(drawingContext);

                ObjectImp *o =
static_cast<ObjectImp*>(contextObject->_fillStyle.imp());
                Gradient *gradient = static_cast<Gradient*>(o);
                CGShadingRef shading = gradient->getShading();
                CGContextDrawShading(drawingContext, shading);
                
                CGContextRestoreGState(drawingContext);
            }
            else
                CGContextFillPath (drawingContext);

            renderer->setNeedsImageUpdate();
            break;
        }

Path creation should be safe as the existing call to CGContextFillRect() clears
the current path.

[David Carson]

Although, with the change specified below applied, it does not produce the right
gradient. It seems that when the color stops are sorted using the qsort
algorithm, the order of the colorstops become:
  lingrad.addColorStop(0, '#00ABEB');
  lingrad.addColorStop(0.5, '#66CC00');
  lingrad.addColorStop(0.5, '#fff');
  lingrad.addColorStop(1, '#fff'); 
(This might be because the way qsort works when the comparison values are
equal.) This results in a gradient from blue to green for the first 50% of the
rectangle and a white area (ie white to white gradient) for the second 50% of
the rectangle.

[David Carson]

Created attachment 6871 [details]
Patch fillrect to support gradients

As CGContextFillRect function does not support filling with a defined gradient, this patch creates a path for the rectangle when a gradient is specified for the fill type.

[Darin Adler]

Comment on attachment 6871 [details]
Patch fillrect to support gradients

Looks great!

r=me.

Marking review- because this needs a change log entry and a test case (presumably differing only in the pixel test).

It will be nice when we change canvas to use GraphicsContext, so we can move all this code out of kjs_html.cpp!

[David Carson]

Created attachment 6879 [details]
patch including test and changelogs

Should also move the canvas test cases:
LayoutTests/fast/dom/image-object-in-canvas*
LayoutTests/fast/dom/quadraticCurveTo*
to
LayoutTests/fast/canvas/

[David Carson]

Created attachment 6887 [details]
patch

Updated test case to describe what the canvas image should look like, as requested by maciej

[Darin Adler]

Comment on attachment 6887 [details]
patch

I found a bug that I'll fix while landing.

CGContextClip needs to be called after setting up the path. It was being called before, so we were actually filling the entire canvas, not just the rectangle! The test case shows the problem, because the filled rectangle is too big.

[Darin Adler]

<rdar://problem/4139672> <canvas> element fillRect ignores gradients (6103)