Summary: | GraphicsContext needs to support Gradients and Patterns (to let us get rid of #ifdefs) | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Schulze <krit> | ||||||||||||||||||||||||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | darin, eric, hausmann, hyatt | ||||||||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||||
Hardware: | PC | ||||||||||||||||||||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||||||||||||||||||||
Bug Depends on: | 20520 | ||||||||||||||||||||||||||||||||||
Bug Blocks: | 20543 | ||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Dirk Schulze
2008-08-13 14:47:36 PDT
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.
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.
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.
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. Created attachment 22857 [details]
bad patch
A bad patch to show my problems of implementing a gradient function to GC.
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. 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(); } 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(-)
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(-)
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.
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 :-).
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.
(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. 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.
*** Bug 20520 has been marked as a duplicate of this bug. *** 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. 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.
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. 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(-)
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(-)
I've filed bug 20543 to deal with the SVG fixes which should be possible once this patch lands. 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(-)
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(-)
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
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
Comment on attachment 23042 [details]
Qt and Cairo support from krit (and blind stab @ wx compile support)
Looks sane, r=me
|