Currently, GeneratorGeneratedImage::drawPattern() creates a tile image buffer which is drawn repeatedly to fill the target area. As an optimization, for axis-aligned linear gradients, the image buffer only holds a 1px slice of the gradient which is then re-sampled to the tile size when drawn. The drawPatern() path is quite common in practice (for example painting the background for boxes with a border), so it would be great if we could avoid rasterizing/resampling at this level, and simply pass the gradient info to Skia instead. Skia cannot handle gradient tiling in general, but for axis aligned linear gradients (presumably the most common case) we can emulate tiling by setting a surrogate gradient up in the following manner: * set the start/stop (p0/p1) to match the tile edges * filter out color stops that fall outside the tile range, and adjust offsets for the remaining ones relative to the new normalized (p0,p1) interval * for the first (0.0) and last (1.0) color stops (corresponding to the tile edges), use interpolates values from the original gradient This should produce equivalent results when drawn across the target area - the difference being we now perform a single fill operation and Skia handles the gradient repeat. I'll share a patch shortly.
Created attachment 184312 [details] Patch First pass.
Attachment 184312 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/gradients/linear-tiled-gradients.html', u'LayoutTests/platform/chromium-linux/fast/backgrounds/gradient-background-leakage-2-expected.png', u'LayoutTests/platform/chromium-linux/fast/backgrounds/gradient-background-leakage-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/border-image-gradient-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/border-image-gradient-sides-and-corners-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/linear-tiled-gradients-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/linear-tiled-gradients-expected.txt', u'LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/Generator.h', u'Source/WebCore/platform/graphics/GeneratorGeneratedImage.cpp', u'Source/WebCore/platform/graphics/GeneratorGeneratedImage.h', u'Source/WebCore/platform/graphics/Gradient.cpp', u'Source/WebCore/platform/graphics/Gradient.h', u'Source/WebCore/platform/graphics/skia/GradientSkia.cpp']" exit_code: 1 LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 184312 [details] Patch Attachment 184312 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16077343
Created attachment 184316 [details] Patch Fix the non-Skia builds.
Attachment 184316 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/gradients/linear-tiled-gradients.html', u'LayoutTests/platform/chromium-linux/fast/backgrounds/gradient-background-leakage-2-expected.png', u'LayoutTests/platform/chromium-linux/fast/backgrounds/gradient-background-leakage-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/border-image-gradient-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/border-image-gradient-sides-and-corners-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/linear-tiled-gradients-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/linear-tiled-gradients-expected.txt', u'LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/Generator.h', u'Source/WebCore/platform/graphics/GeneratorGeneratedImage.cpp', u'Source/WebCore/platform/graphics/GeneratorGeneratedImage.h', u'Source/WebCore/platform/graphics/Gradient.cpp', u'Source/WebCore/platform/graphics/Gradient.h', u'Source/WebCore/platform/graphics/skia/GradientSkia.cpp']" exit_code: 1 LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 184568 [details] Patch Trying again just to see whether the check-webkit-style error was transient/bot related.
Attachment 184568 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/gradients/linear-tiled-gradients.html', u'LayoutTests/platform/chromium-linux/fast/backgrounds/gradient-background-leakage-2-expected.png', u'LayoutTests/platform/chromium-linux/fast/backgrounds/gradient-background-leakage-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/border-image-gradient-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/border-image-gradient-sides-and-corners-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/linear-tiled-gradients-expected.png', u'LayoutTests/platform/chromium-linux/fast/gradients/linear-tiled-gradients-expected.txt', u'LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/Generator.h', u'Source/WebCore/platform/graphics/GeneratorGeneratedImage.cpp', u'Source/WebCore/platform/graphics/GeneratorGeneratedImage.h', u'Source/WebCore/platform/graphics/Gradient.cpp', u'Source/WebCore/platform/graphics/Gradient.h', u'Source/WebCore/platform/graphics/skia/GradientSkia.cpp']" exit_code: 1 LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #7) > LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] > Total errors found: 1 in 16 files Not sure what to make of this - I definitely have auto-props enabled and check-webkit-style passes locally: fmalita@fmalita-linux2:~/wk$ Tools/Scripts/check-webkit-style Total errors found: 0 in 16 files fmalita@fmalita-linux2:~/wk$ git svn propget "svn:mime-type" LayoutTests/platform/chromium-linux/fast/gradients/simple-gradients-expected.png image/png Since the tool is looking at the local svn config, I'm guessing this is either a check-webkit-style bug or bot misconfiguration.
(In reply to comment #0) > Currently, GeneratorGeneratedImage::drawPattern() creates a tile image buffer which is drawn repeatedly to fill the target area. As an optimization, for axis-aligned linear gradients, the image buffer only holds a 1px slice of the gradient which is then re-sampled to the tile size when drawn. I see a problem with this: Taking a 1px slice could result in stripe artifacts due to dithering. Any way to make the tile generator produce 2px slices instead?
I believe the patch is meant to use skia's native gradient drawing more, and use constructed-bitmaps less. The result should be - better behavior/utilization w/ pictures - better or equivalent quality - better or equivalent speed
Skia currently has an optimized path for 1xN textures. Making them 2xN would block this fast path but, as Mike said, this CL is all about not even converting to a 1xN texture in the first place.
(In reply to comment #10) > I believe the patch is meant to use skia's native gradient drawing more, and use constructed-bitmaps less. The result should be > - better behavior/utilization w/ pictures > - better or equivalent quality > - better or equivalent speed After a closer look at the code, I tend to agree. The description was misleading. If a 1px slice ends up screwing with dithering, it would be skia's fault.
Created attachment 187093 [details] Patch Rebased. Hopefully the style bot has also come to its senses.
Created attachment 187329 [details] Patch Updated after the Skia flags changes.
Comment on attachment 187329 [details] Patch Attachment 187329 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16434703 New failing tests: http/tests/cache/cached-main-resource.html
Comment on attachment 187329 [details] Patch Clearing flags on Skia-specific patch.
Please don't WontFix bugs until we've had a chance to migrate them. It will only be a few days at most before we've finished that.
Sorry, I have no idea what you are talking about. I don’t know what migrating is, how to tell if a bug is migrated, or why marking them as RESOLVED WONTFIX gets in the way of migration. Could you post some instructions about this somewhere?
(In reply to comment #18) > Sorry, I have no idea what you are talking about. I don’t know what migrating is, how to tell if a bug is migrated, or why marking them as RESOLVED WONTFIX gets in the way of migration. Could you post some instructions about this somewhere? Sorry for not making things clearer. There is a chain on WebKit dev "Chromium bugs marked as WontFix" but it is not explicit about the procedure. We were trying to minimize the impact on WebKit developers - you shouldn't have to spend time helping us close out our bugs. The goal is to have no unresolved Chromium-only issues in WebKit while not losing any information for Chrome. To achieve that, we (mostly I) are searching for all unresolved bugs containing "Chromium" or "Skia" and using a Google internal tool to migrate them to crbug. It will take some time as we have no automagical way of deciding if a bug really needs to move across. Maybe a few days. I can't post the tool because it's a Chrome extension on the internal network. Believe me, I wish I could get more people involved. The only way we can avoid losing track of bugs is if they remain unresolved while we look at them. There's no way we can check resolved issues - there are too many. To speed things along I am doing absolutely minimal modifications to the WebKit bugs. The only way to see if a bug has been migrated is to go to crbug.com and search for "label:WebKit-ID-XXXXX" where XXXXX is the WebKit bug number. Email has also gone out to all those people with WebKit bugs related to Chromium that have a patch pending review. We will be doing what you're doing (clearing flags) as soon as we can safely do so.