Bug 20373 – GraphicsContext needs to support Gradients and Patterns (to let us get rid of #ifdefs)

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Schulze]

This are only some idears, which need the patches of https://bugs.webkit.org/show_bug.cgi?id=20351. The main idear is to avoid
#ifdefs and to draw Canvas by GraphicsContext.

Changes in CanvasRenderingContext2D:
void CanvasRenderingContext2D::fillRect(float x, float y, float width, float height)
{
    if (!validateRectForCanvas(x, y, width, height))
        return;

    GraphicsContext* c = drawingContext();
    if (!c)
        return;

    FloatRect rect(x, y, width, height);
    willDraw(rect);

    if (state().m_fillStyle->canvasGradient()) {
        applyFillGradient();
    else if (state().m_fillStyle->pattern())
        applyFillPattern();
    else
        c->fillRect(rect);       // a new function in GC, should use fillColor()
}
// the same for strokeRect()

//like applyFillPattern() in the patch https://bugs.webkit.org/attachment.cgi?id=22764
void CanvasRenderingContext2D::applyFillGradient()
{
    GraphicsContext* c = drawingContext();
    if (!c)
        return;

    // needs some convertations to match or use CanvasGradient* and add an extra function to GC
    CanvasPattern* pattern = state().m_fillStyle->canvasGradient()->gradient();
    if (!pattern)
        return;

    c->applyFillPattern(pattern->pattern());
    state().m_appliedFillGradient = true;
}

We could add a function for pathes strokePath() in GC to draw lines and shapes by GC.
I'll make a patch later.

[Dirk Schulze]

Created attachment 22798 [details]
Canvas fillRect

An example for canvas fillRect(); It works for cairo, but I'm not sure if it works for Cg too. I haven't modified Qt yet.

[Dirk Schulze]

Created attachment 22823 [details]
Canvas-Clean-up

It doesn't work very well. I submitted it to show the problems.
Cairo and Cg differs in many things and it's difficult to make the draw-operation plattform-independent.

The main-problem is, that in Cg you set the fill/stroke color to the beginning and in Cairo short before the draw-operation (because Cairo makes no differnce between fill and stroke colors).

I tried to re-use the concept of my first idear, but run in to problems by the fill- and stroke-operation with patterns. strokeRect differs too stroke and fillRect differs to fill a bit to much. Not sure if the first idear was good enough, because you have to insert more (sometimes nearly senseless) functions to GraphicsContext to make it work.

[Dirk Schulze]

Created attachment 22841 [details]
Canvas CleanUp

Added some functions to GraphisContext to make it platform-independent. Didn't find a way to limit it to less functions because of the problems I discribed in the posting before.

[Eric Seidel (no email)]

Wow.  I think this is a really great start.  I think I need to think a little bit more about what methods should be added to GraphicsContext and which should be added to Pattern or Gradient instead, and what methods on GraphicsContext should take a Pattern* or Gradient* or if GraphicsContext should hold a Pattern/Gradient as part of its current state.  But this is a fantastic start.

Lets talk about this more over irc on monday.

[Dirk Schulze]

Created attachment 22857 [details]
bad patch

A bad patch to show my problems of implementing a gradient function to GC.

[Eric Seidel (no email)]

We talked about this over IRC.  I propose we move to the following model:

void setStokePattern(const Pattern&);
void setStokeGradient(const Gradient&);

void setFillPattern(const Pattern&);
void setFillGradient(const Gradient&);

void fillPath();
void strokePath();
void drawPath(); // fills and strokes, convenience and speed.  CG has a fast-path for this.

// more convenience and speed.  CG has a fast-path for rects-only (callers also don't have to bother with Path objects then)
void fillRect(const FloatRect&);
void strokeRect(const FloatRect&);
void drawRect(const FloatRect&);


Internally, the GraphicsContextState stack gets some extra variables:

Pattern m_fillPattern;
Gradient m_fillGradient;
Pattern m_strokePattern;
Gradient m_strokeGradient;

enum ColorSpace {
  SolidColorSpace,
  PatternColorSpace,
  GradientColorSpace
}

ColorSpace m_fillColorSpace;
ColorSpace m_strokeColorSpace;

I am slightly concerned that adding 2 patterns and 2 gradient objects to each GraphicsContext state might make them more expensive to work with.   Especially since each new state inherits from the previous.  We may need to move to some sort of RefCounted model for Pattern and Gradient over time.

[Eric Seidel (no email)]

As we talked about over IRC, since Cairo doesn't support saving separate stroke and fill colors on the context, it obviously doesn't have a "draw" fast path.

Cairo GraphicsContext code needs to move to this model anyway:

someFillMethod()
{
// grab the current fill color off of the GraphicContextState
// set it as the "cairo source"
// do the actual fill
}

instead of how CG works, which is to just pipe the "set fill color" calls straight down into the CoreGraphics context.

Also, because Cairo doesn't have a built-in fast-path for "draw", Cairo (and any other graphics library which doesn't support simultaneous fill + stroke) can just write:

GraphicsContext::drawPath()
{
    fillPath();
    strokePath();
}

[Eric Seidel (no email)]

Created attachment 22934 [details]
A start on the patch for krit

 WebCore/WebCore.xcodeproj/project.pbxproj          |    4 +
 .../js/JSCanvasRenderingContext2DCustom.cpp        |    4 +-
 WebCore/css/CSSGradientValue.cpp                   |   10 +-
 WebCore/css/CSSGradientValue.h                     |    2 +-
 WebCore/html/CanvasGradient.cpp                    |    4 +-
 WebCore/html/CanvasGradient.h                      |    8 +-
 WebCore/html/CanvasPattern.cpp                     |    2 +-
 WebCore/html/CanvasPattern.h                       |    6 +-
 WebCore/html/CanvasRenderingContext2D.cpp          |  195 +-------------------
 WebCore/html/CanvasRenderingContext2D.h            |    6 -
 WebCore/html/CanvasStyle.cpp                       |    6 +
 WebCore/html/CanvasStyle.h                         |    3 +-
 WebCore/platform/graphics/GeneratedImage.h         |   10 +-
 WebCore/platform/graphics/Generator.h              |    4 +-
 WebCore/platform/graphics/Gradient.h               |   14 ++-
 WebCore/platform/graphics/GraphicsContext.cpp      |   38 +++-
 WebCore/platform/graphics/GraphicsContext.h        |   38 +++--
 WebCore/platform/graphics/GraphicsContextPrivate.h |   31 +++-
 WebCore/platform/graphics/Pattern.h                |   15 +-
 .../graphics/cairo/GraphicsContextCairo.cpp        |   17 ++-
 WebCore/platform/graphics/cg/GraphicsContextCG.cpp |   39 ++--
 WebCore/platform/graphics/qt/GraphicsContextQt.cpp |   28 ++--
 WebCore/platform/graphics/wx/GraphicsContextWx.cpp |   16 +-
 23 files changed, 204 insertions(+), 296 deletions(-)

[Eric Seidel (no email)]

Created attachment 22940 [details]
A bit further with the patch for krit

 WebCore/WebCore.xcodeproj/project.pbxproj          |    4 +
 .../js/JSCanvasRenderingContext2DCustom.cpp        |    4 +-
 WebCore/css/CSSGradientValue.cpp                   |   10 +-
 WebCore/css/CSSGradientValue.h                     |    2 +-
 WebCore/html/CanvasGradient.cpp                    |    4 +-
 WebCore/html/CanvasGradient.h                      |    8 +-
 WebCore/html/CanvasPattern.cpp                     |    2 +-
 WebCore/html/CanvasPattern.h                       |    6 +-
 WebCore/html/CanvasRenderingContext2D.cpp          |  195 +-------------------
 WebCore/html/CanvasRenderingContext2D.h            |    6 -
 WebCore/html/CanvasStyle.cpp                       |    6 +
 WebCore/html/CanvasStyle.h                         |    3 +-
 WebCore/platform/graphics/GeneratedImage.h         |   10 +-
 WebCore/platform/graphics/Generator.h              |    4 +-
 WebCore/platform/graphics/Gradient.h               |   14 ++-
 WebCore/platform/graphics/GraphicsContext.cpp      |   38 +++-
 WebCore/platform/graphics/GraphicsContext.h        |   38 +++--
 WebCore/platform/graphics/GraphicsContextPrivate.h |   33 +++-
 WebCore/platform/graphics/Path.h                   |    8 +-
 WebCore/platform/graphics/Pattern.h                |   15 +-
 .../graphics/cairo/GraphicsContextCairo.cpp        |   17 ++-
 WebCore/platform/graphics/cg/GraphicsContextCG.cpp |  107 ++++++++++--
 WebCore/platform/graphics/qt/GraphicsContextQt.cpp |   28 ++--
 WebCore/platform/graphics/wx/GraphicsContextWx.cpp |   16 +-
 24 files changed, 281 insertions(+), 297 deletions(-)

[Dirk Schulze]

Created attachment 22946 [details]
update of GCCairo

The second part of the patch. For Cairo this time. setPlatformStroke* and setPlatformFill* are not included as methods (they perhaps don't get it to the final patch).

Gradients and patterns are black. Seems to be a problem with the patch before (but Gradients in CSS work) and should also happen to the other platforms.

[Dirk Schulze]

Created attachment 22950 [details]
Colors from future

Canvas takes fillSytle() from the future, if the current fillStyle() is broken.

The attachement discribes the problem: There should be 3 rects. The first is the one on the right/bottom. And should be red.
The second rect on the right/top should be filled with fillStyle = "#f00)"; but fillstyle is broken, therefore it should use the old fillstyle of the first rect (webkit without the patch fill it transparent).
And after the fill-operation of the second rect I initialise a new fillstyle, blue (this time valid). And the third rect on the left side should be blue.

But with the patch, the rect with the invalid fillStyle gets rendered blue too. Webkit has an oracle now and can use fillstyles in the future :-).

[Dirk Schulze]

Created attachment 22951 [details]
Combination of the last 2 patches

This is only a combination of the last 2 patches, as well as a fix of gradients and pattern.
"stroke" and "fill" was mixed up in CanvasStyle.cpp.

The fillStyle() (as well as strokeStyle()) problem discribed above can perhaps  be solved in another bug-report, because the way it is handled now is wrong too.

[Dirk Schulze]

(In reply to comment #12)
> as well as a fix of gradients and pattern.
> "stroke" and "fill" was mixed up in CanvasStyle.cpp.
the same in GraphicsContextCg.cpp but wasn't changed by this patch.

[Dirk Schulze]

Created attachment 22984 [details]
Qt implementation

I tried to implement the changes to Qt but get Segmentation Fault on gradients and patterns. Perhaps a pointer-problem.

[Dirk Schulze]

*** Bug 20520 has been marked as a duplicate of this bug. ***

[Dirk Schulze]

Created attachment 23000 [details]
Fixes Cg

This patch fixes bugs in CanvasStyle, GraphicsContextCg, RadialGradientAttribute. Now the Cg-implementation should be able to display Canvas as well as SVG-Gradients.
I fixed Cairo too. But the modification causes some problems, eg. http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-pservers-grad-16-b.html -> Segmentation Fault (The gradient with the false stopColor) and I can't destroy the pattern for drawing gradients, as long as the gradient is not destroied.
I made a build-fix for Qt but haven't updated SVG yet. gradients and patterns on Canvas still end up in an Segmentation Fault. Perhaps some of the Qt-folks can help out.

The big patch should be splitted in to different patches.

[Eric Seidel (no email)]

Comment on attachment 23000 [details]
Fixes Cg

We should talk about this over IRC.  I'm not sure I want to include the SVG gradient/pattern re-write in the first landed patches.  Everything is still separate in my git tree.

[Eric Seidel (no email)]

Ok, I have a final working patch which passes all the <canvas> test cases I could find (at least as many as WebKit TOT does).  Will upload shortly.

[Eric Seidel (no email)]

Created attachment 23023 [details]
Add stroke/fill Gradient and Pattern support to GraphicsContext and update <canvas> to use it.

 WebCore/WebCore.xcodeproj/project.pbxproj          |    4 +
 .../js/JSCanvasRenderingContext2DCustom.cpp        |    4 +-
 WebCore/css/CSSGradientValue.cpp                   |   10 +-
 WebCore/css/CSSGradientValue.h                     |    2 +-
 WebCore/html/CanvasGradient.cpp                    |    4 +-
 WebCore/html/CanvasGradient.h                      |    8 +-
 WebCore/html/CanvasPattern.cpp                     |    2 +-
 WebCore/html/CanvasPattern.h                       |    6 +-
 WebCore/html/CanvasRenderingContext2D.cpp          |  196 +-------------------
 WebCore/html/CanvasRenderingContext2D.h            |    6 -
 WebCore/html/CanvasStyle.cpp                       |    7 +
 WebCore/html/CanvasStyle.h                         |    3 +-
 WebCore/platform/graphics/GeneratedImage.h         |   10 +-
 WebCore/platform/graphics/Generator.h              |    4 +-
 WebCore/platform/graphics/Gradient.h               |   16 ++-
 WebCore/platform/graphics/GraphicsContext.cpp      |   68 ++++++-
 WebCore/platform/graphics/GraphicsContext.h        |   39 +++--
 WebCore/platform/graphics/GraphicsContextPrivate.h |   33 +++-
 WebCore/platform/graphics/GraphicsTypes.h          |    2 +-
 WebCore/platform/graphics/Path.h                   |    8 +-
 WebCore/platform/graphics/Pattern.h                |   15 +-
 .../graphics/cairo/GraphicsContextCairo.cpp        |   17 ++-
 WebCore/platform/graphics/cg/GraphicsContextCG.cpp |  173 ++++++++++++++---
 WebCore/platform/graphics/qt/GraphicsContextQt.cpp |   28 ++--
 WebCore/platform/graphics/wx/GraphicsContextWx.cpp |   16 +-
 25 files changed, 366 insertions(+), 315 deletions(-)

[Eric Seidel (no email)]

Created attachment 23024 [details]
Qt and Cairo support from krit

 .../graphics/cairo/GraphicsContextCairo.cpp        |   96 ++++++++++++++-----
 WebCore/platform/graphics/qt/GraphicsContextQt.cpp |  100 +++++++++++++++++++-
 2 files changed, 166 insertions(+), 30 deletions(-)

[Eric Seidel (no email)]

I've filed bug 20543 to deal with the SVG fixes which should be possible once this patch lands.

[Eric Seidel (no email)]

Created attachment 23041 [details]
Add fill/stroke Pattern and Gradient support to GraphicsContext (now with ChangeLog)

 WebCore/ChangeLog                                  |   84 +++++++++
 WebCore/WebCore.xcodeproj/project.pbxproj          |    4 +
 .../js/JSCanvasRenderingContext2DCustom.cpp        |    4 +-
 WebCore/css/CSSGradientValue.cpp                   |   10 +-
 WebCore/css/CSSGradientValue.h                     |    2 +-
 WebCore/html/CanvasGradient.cpp                    |    4 +-
 WebCore/html/CanvasGradient.h                      |    8 +-
 WebCore/html/CanvasPattern.cpp                     |    2 +-
 WebCore/html/CanvasPattern.h                       |    6 +-
 WebCore/html/CanvasRenderingContext2D.cpp          |  196 +-------------------
 WebCore/html/CanvasRenderingContext2D.h            |    6 -
 WebCore/html/CanvasStyle.cpp                       |    7 +
 WebCore/html/CanvasStyle.h                         |    3 +-
 WebCore/platform/graphics/GeneratedImage.h         |   10 +-
 WebCore/platform/graphics/Generator.h              |    4 +-
 WebCore/platform/graphics/Gradient.h               |   16 ++-
 WebCore/platform/graphics/GraphicsContext.cpp      |   68 ++++++-
 WebCore/platform/graphics/GraphicsContext.h        |   39 +++--
 WebCore/platform/graphics/GraphicsContextPrivate.h |   33 +++-
 WebCore/platform/graphics/GraphicsTypes.h          |    2 +-
 WebCore/platform/graphics/Path.h                   |    8 +-
 WebCore/platform/graphics/Pattern.h                |   15 +-
 WebCore/platform/graphics/cg/GraphicsContextCG.cpp |  173 ++++++++++++++---
 23 files changed, 415 insertions(+), 289 deletions(-)

[Eric Seidel (no email)]

Created attachment 23042 [details]
Qt and Cairo support from krit (and blind stab @ wx compile support)

 WebCore/ChangeLog                                  |   46 +++++++
 .../graphics/cairo/GraphicsContextCairo.cpp        |  121 ++++++++++++++-----
 WebCore/platform/graphics/qt/GraphicsContextQt.cpp |  133 ++++++++++++++++----
 WebCore/platform/graphics/wx/GraphicsContextWx.cpp |   48 ++++++-
 4 files changed, 289 insertions(+), 59 deletions(-)

[Oliver Hunt]

Comment on attachment 23041 [details]
Add fill/stroke Pattern and Gradient support to GraphicsContext (now with ChangeLog)

WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp
if (style->canvasPattern())
    return toJS(exec, style->canvasPattern());

I'd prefer if (c=style->canvasPattern()) return toJS(exec, c)

Many comments that shouldn't be landed 
// TODO(eseidel): This is a place-holder until some day wh... <-- badness

[Oliver Hunt]

Comment on attachment 23041 [details]
Add fill/stroke Pattern and Gradient support to GraphicsContext (now with ChangeLog)

r=me, assuming you replace your TODO()'s with bug#'s

[Oliver Hunt]

Comment on attachment 23042 [details]
Qt and Cairo support from krit (and blind stab @ wx compile support)

Looks sane, r=me

[Eric Seidel (no email)]

r35966 r35967