[Webkit-unassigned] [Bug 108763] Canvas GraphicsContext miterLimit initializes to 4 (SVG) instead of 10
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Feb 16 00:16:40 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=108763
--- Comment #22 from Jonathan Olson <olsonsjc at gmail.com> 2013-02-16 00:18:58 PST ---
(In reply to comment #21)
> (From update of attachment 186469 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186469&action=review
>
> > Source/WebCore/ChangeLog:8
> > + Canvas GraphicsContext miterLimit initializes to 4 (SVG) instead of 10
> > + https://bugs.webkit.org/show_bug.cgi?id=108763
> > +
> > + Reviewed by NOBODY (OOPS!).
> > +
> > + Test: fast/canvas/canvas-miterLimit.html
>
> Can you be a bit more explicit and add more description please? A changelog should contain a description of the changes and why you changed it.
Sorry, I had tried to add the following changelog entries, but it appears it was wiped out in the latest patch. I'll submit a new patch with the tweaks recommended, and with fixed changelog entries. They were as below:
(Source/WebCore/ChangeLog)
+2013-02-04 Jonathan Olson <olsonsjc at gmail.com>
+
+ Canvas GraphicsContext miterLimit initializes to 4 (SVG) instead of 10
+ https://bugs.webkit.org/show_bug.cgi?id=108763
+
+ Reviewed by NOBODY (OOPS!).
+
+ Certain GraphicsContexts (namely PlatformContextSkia) are initialized with some line style
+ attributes (in this case miterLimit) that are different from the Canvas defaults, and
+ CanvasRenderingContext2D did not properly initialize these values. Creating a ContextRenderingContext2D
+ (in JS with getCanvas( '2d ') ) and drawing would use an improper miterLimit of 4 (SVG default)
+ instead of the Canvas default of 10. This change adds applyLineStyles() which properly initializes
+ these values in the GraphicsContext on CanvasRenderingContext2D construction. It includes more
+ than just the miterLimit as a safety check.
and
(LayoutTests/ChangeLog)
+2013-02-04 Jonathan Olson <olsonsjc at gmail.com>
+
+ Canvas GraphicsContext miterLimit initializes to 4 (SVG) instead of 10
+ https://bugs.webkit.org/show_bug.cgi?id=108763
+
+ Reviewed by NOBODY (OOPS!).
+
+ Certain GraphicsContexts (namely PlatformContextSkia) are initialized with some line style
+ attributes (in this case miterLimit) that are different from the Canvas defaults, and
+ CanvasRenderingContext2D did not properly initialize these values. Creating a ContextRenderingContext2D
+ (in JS with getCanvas( '2d ') ) and drawing would use an improper miterLimit of 4 (SVG default)
+ instead of the Canvas default of 10. This change adds applyLineStyles() which properly initializes
+ these values in the GraphicsContext on CanvasRenderingContext2D construction. It includes more
+ than just the miterLimit as a safety check.
+
+ canvas-incremental-repaint-expected.html included a line join that has an angle that is beveled with
+ a miterLimit of 4 but mitered with a miterLimit of 10, so I've removed the incorrectly-beveled test
+ PNG cases (predominantly chromium, due to Skia).
>
> > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:139
> > + applyLineStyles(); // Ensure the GraphicsContext is initialized with consistent line styles, as values like miterLimit may be different.
>
> comments in the line before.
>
> > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:599
> > +void CanvasRenderingContext2D::applyLineStyles() const
>
> It is not called anywhere else, an inline function where you pass the Graphics context and state might be enough.
Or would it be better to just inline this inside the one call site (in the CanvasRenderingContext2D constructor?)
>
> > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:608
> > + c->setStrokeThickness(state().m_lineWidth);
> > + c->setLineCap(state().m_lineCap);
> > + c->setLineJoin(state().m_lineJoin);
> > + c->setMiterLimit(state().m_miterLimit);
> > +}
>
> Are there problems with the other default value as well? If so, can you add more tests to cover that please?
I am not aware of any problems besides the miterLimit issue, I was just trying to be safe by keeping that initialization inside CanvasRenderingContext2D, instead of relying on each GraphicsContext's default to remain forever the same as the Canvas spec requires.
>
> > LayoutTests/ChangeLog:4
> > + Canvas GraphicsContext miterLimit initializes to 4 (SVG) instead of 10
> > + https://bugs.webkit.org/show_bug.cgi?id=108763
>
> Please add one or two lines of description.
>
> > LayoutTests/ChangeLog:18
> > + * platform/chromium-android/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/chromium-linux-x86/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/chromium-linux/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/chromium-mac-lion/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/chromium-mac-snowleopard/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/chromium-mac/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/chromium-win-xp/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/chromium-win/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
> > + * platform/mac-snowleopard/fast/canvas/canvas-incremental-repaint-expected.png: Removed.
>
> Is this cleanup? This should be mentioned in the change log as well.
This will also be fixed in the changelog as noted above.
>
> > LayoutTests/fast/canvas/canvas-miterLimit-expected.txt:5
> > +CONSOLE MESSAGE: line 19: The following alpha should be 255, since the unit rectangle is completely in the mitered join bounds.
> > +CONSOLE MESSAGE: line 19: Since the drawn angle is 13 degrees, a mitered join will be drawn if miterLimit is above 8.833671471997569.
> > +CONSOLE MESSAGE: line 19: The default CanvasRenderingContext2D miterLimit value is 10, but some GraphicsContexts are created with a default of 4.
> > +CONSOLE MESSAGE: line 19: If miterLimit is initialized incorrectly, this will render as a bevel join instead and will be transparent (0).
> > +CONSOLE MESSAGE: line 19: imageData.data[3] = 255
>
> We don't use write() and test with console messages. Please look at other tests in the folder like canvas-clearRect.
Ok, thanks! I'll fix this.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list