Add support for command line flag enable-per-tile-drawing in DumpRenderTree.
Created attachment 118250 [details] Patch
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment on attachment 118250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118250&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:2906 > + ccSettings.perTilePainting = page()->settings()->perTileDrawingEnabled(); is it per-tile drawing or per-tile painting? i thought we made a careful distinction between those terms. seeing them equated here makes me confused.
(In reply to comment #3) > (From update of attachment 118250 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=118250&action=review > > > Source/WebKit/chromium/src/WebViewImpl.cpp:2906 > > + ccSettings.perTilePainting = page()->settings()->perTileDrawingEnabled(); > > is it per-tile drawing or per-tile painting? > > i thought we made a careful distinction between those terms. seeing > them equated here makes me confused. It's currently "accelerated painting" in the compositor but "accelerated drawing" outside. This change makes the "per-tile" option consistent with the "accelerated" option. If we want "painting" to be used everywhere, both of these options should change.
Comment on attachment 118250 [details] Patch Attachment 118250 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10752277 New failing tests: compositing/plugins/invalidate_rect.html
Comment on attachment 118250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118250&action=review Can you look into the test failure? > Source/WebCore/page/Settings.h:504 > + void setPerTileDrawingEnabled(bool enabled) { m_perTileDrawingEnabled = enabled; } > + bool perTileDrawingEnabled() const { return m_perTileDrawingEnabled; } this should be painting, methinks >>> Source/WebKit/chromium/src/WebViewImpl.cpp:2906 >>> + ccSettings.perTilePainting = page()->settings()->perTileDrawingEnabled(); >> >> is it per-tile drawing or per-tile painting? >> >> i thought we made a careful distinction between those terms. seeing >> them equated here makes me confused. > > It's currently "accelerated painting" in the compositor but "accelerated drawing" outside. This change makes the "per-tile" option consistent with the "accelerated" option. If we want "painting" to be used everywhere, both of these options should change. "accelerated drawing" exists only in cross-platform code because it's the term that apple likes to use for what we call accelerated painting. We should continue to use that term in cross-platform code where it's necessary, but stick to painting everywhere else > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:223 > + optparse.make_option("--per-tile-drawing", this also should be painting > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:225 > + help="Use per-tile drawing of composited pages"), painting
(In reply to comment #6) > (From update of attachment 118250 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=118250&action=review > > Can you look into the test failure? > > > Source/WebCore/page/Settings.h:504 > > + void setPerTileDrawingEnabled(bool enabled) { m_perTileDrawingEnabled = enabled; } > > + bool perTileDrawingEnabled() const { return m_perTileDrawingEnabled; } > > this should be painting, methinks > > >>> Source/WebKit/chromium/src/WebViewImpl.cpp:2906 > >>> + ccSettings.perTilePainting = page()->settings()->perTileDrawingEnabled(); > >> > >> is it per-tile drawing or per-tile painting? > >> > >> i thought we made a careful distinction between those terms. seeing > >> them equated here makes me confused. > > > > It's currently "accelerated painting" in the compositor but "accelerated drawing" outside. This change makes the "per-tile" option consistent with the "accelerated" option. If we want "painting" to be used everywhere, both of these options should change. > > "accelerated drawing" exists only in cross-platform code because it's the term that apple likes to use for what we call accelerated painting. We should continue to use that term in cross-platform code where it's necessary, but stick to painting everywhere else > > > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:223 > > + optparse.make_option("--per-tile-drawing", > > this also should be painting > > > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:225 > > + help="Use per-tile drawing of composited pages"), > > painting OK, no problem. I'll change this patch so that every chromium specific use of "accelerated/per-tile drawing" becomes "accelerated/per-tile painting" instead. This also means changing the chromium command line option "--enable-accelerated-drawing" to "--enable-accelerated-painting", which I'll take care of while adding the "--enable-per-tile-painting" option.
DRT uses accelerated-drawing, which I think is appropriate because it is changing the cross-platform accelerated-drawing setting. By the same argument, --enable-accelerated-drawing flag is appropriate for chrome? I prefer using one name everywhere. I do not care which one it is. If we hate "drawing" so much, can we persuade Apple engineers to use "painting"?
Started changing to "painting" where possible but think this makes it more confusing than the current patch unless cross-platform code is also changed to "painting". The current patch makes it clear where "drawing" vs "painting" is used so I think we should stick with it for now until we can convince apple to use "painting" instead.
I disagree. We should keep all chromium-specific code consistent, at the very least.
Created attachment 118464 [details] Patch
Comment on attachment 118464 [details] Patch R=me
Comment on attachment 118464 [details] Patch Clearing flags on attachment: 118464 Committed r102400: <http://trac.webkit.org/changeset/102400>
All reviewed patches have been landed. Closing bug.