Currently, the imageSmoothingEnabled is a property of the canvas. As per a recent update of the spec, this flag should be a property of a drawing state.
Created attachment 144903 [details] Patch
Comment on attachment 144903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144903&action=review > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2283 > + modifiableState().m_imageSmoothingEnabled = enabled; Could you put this line right below the realizeSaves() call? Most other state modifiers seem to do that. > LayoutTests/fast/canvas/canvas-imageSmoothingEnabled-expected.txt:10 > +Test restore works. We save a false state; create, then save a true state; and then finally restore. Nit: "Test restore works" -> "Test that restore works". > LayoutTests/fast/canvas/canvas-imageSmoothingEnabled-expected.txt:25 > +Test restoring actually changes smoothing and not just the attribute value. We are restoring to a point where imageSmoothingEnabled is set to false. Nit: "Test restoring" -> "Test that restoring" > LayoutTests/fast/canvas/script-tests/canvas-imageSmoothingEnabled.js:11 > +debug("Test restore works. We save a false state; create, then save a true state; and then finally restore."); Same as above. > LayoutTests/fast/canvas/script-tests/canvas-imageSmoothingEnabled.js:68 > +debug("Test restoring actually changes smoothing and not just the attribute value. We are restoring to a point where imageSmoothingEnabled is set to false."); Same as above.
Created attachment 144938 [details] Patch
Comment on attachment 144938 [details] Patch Looks good (assuming the bots are ok with it). r=me
Comment on attachment 144938 [details] Patch Clearing flags on attachment: 144938 Committed r119100: <http://trac.webkit.org/changeset/119100>
All reviewed patches have been landed. Closing bug.
This is failing on the GTK bots for some reason, even if it looks like something that should work in a cross-platform basis. Any idea? The diff: --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/canvas/canvas-imageSmoothingEnabled-expected.txt +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/canvas/canvas-imageSmoothingEnabled-actual.txt @@ -23,9 +23,9 @@ PASS left_of_center_pixel.data[1] !== 0 is true PASS left_of_center_pixel.data[2] !== 0 is true Test that restoring actually changes smoothing and not just the attribute value. We are restoring to a point where imageSmoothingEnabled is set to false. -PASS left_of_center_pixel.data[0] is 0 -PASS left_of_center_pixel.data[1] is 0 -PASS left_of_center_pixel.data[2] is 0 +FAIL left_of_center_pixel.data[0] should be 0. Was 63. +FAIL left_of_center_pixel.data[1] should be 0. Was 63. +FAIL left_of_center_pixel.data[2] should be 0. Was 63. PASS successfullyParsed is true TEST COMPLETE
(In reply to comment #7) > This is failing on the GTK bots for some reason, even if it looks like something that should work in a cross-platform basis. > > Any idea? > > The diff: > --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/canvas/canvas-imageSmoothingEnabled-expected.txt > +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/canvas/canvas-imageSmoothingEnabled-actual.txt > @@ -23,9 +23,9 @@ > PASS left_of_center_pixel.data[1] !== 0 is true > PASS left_of_center_pixel.data[2] !== 0 is true > Test that restoring actually changes smoothing and not just the attribute value. We are restoring to a point where imageSmoothingEnabled is set to false. > -PASS left_of_center_pixel.data[0] is 0 > -PASS left_of_center_pixel.data[1] is 0 > -PASS left_of_center_pixel.data[2] is 0 > +FAIL left_of_center_pixel.data[0] should be 0. Was 63. > +FAIL left_of_center_pixel.data[1] should be 0. Was 63. > +FAIL left_of_center_pixel.data[2] should be 0. Was 63. > PASS successfullyParsed is true > > TEST COMPLETE Is it possible that GTK's (cairo's?) implementation of GraphicsContext does not save/restore the ImageInterpolation correctly?
(In reply to comment #8) > (In reply to comment #7) > > This is failing on the GTK bots for some reason, even if it looks like something that should work in a cross-platform basis. > > > > Any idea? > > > > The diff: > > --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/canvas/canvas-imageSmoothingEnabled-expected.txt > > +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/fast/canvas/canvas-imageSmoothingEnabled-actual.txt > > @@ -23,9 +23,9 @@ > > PASS left_of_center_pixel.data[1] !== 0 is true > > PASS left_of_center_pixel.data[2] !== 0 is true > > Test that restoring actually changes smoothing and not just the attribute value. We are restoring to a point where imageSmoothingEnabled is set to false. > > -PASS left_of_center_pixel.data[0] is 0 > > -PASS left_of_center_pixel.data[1] is 0 > > -PASS left_of_center_pixel.data[2] is 0 > > +FAIL left_of_center_pixel.data[0] should be 0. Was 63. > > +FAIL left_of_center_pixel.data[1] should be 0. Was 63. > > +FAIL left_of_center_pixel.data[2] should be 0. Was 63. > > PASS successfullyParsed is true > > > > TEST COMPLETE > > Is it possible that GTK's (cairo's?) implementation of GraphicsContext does not save/restore the ImageInterpolation correctly? Yes, it looks like PlatformContextCairo stores the interpolation quality directly, and it should be moved to PlatformContextCairo::State instead. That would be my guess (assuming the GTK port uses Cairo).
Thanks for the feedback. Unfortunately, I've tried that suggestion locally and didn't seem to work, but it's mostly my fault, since I'm not an expert in the graphics part and I'm most likely doing it wrong. Anyway, I will file a bug for this issue, add the test to test_expectations.txt and add the right people to CC, pointing back to this bug. Thanks
Bug for tracking the issue down for GTK: bug 87985