Bug 109658 – Unify GraphicsLayer::setContentsToMedia and setContentsToCanvas

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Tien-Ren Chen]

Per discussion in https://bugs.webkit.org/show_bug.cgi?id=109560
trchen: Per off-line discussion with aelias@, we think we should make a GraphicsLayerChromium::setContentsToCoordinatedScrollbar extension. Does that sound cleaner?
smfr:  You could do that, but it might be better to make a more general setContentsToCustomSomething() function that can live on GraphicsLayer.
jamesr: As far as I can see, implementations of GraphicsLayer do not do anything different for setContentsToMedia(PlatformLayer*) and setContentsToCanvas(PlatformLayer*). GraphicsLayerCA and GraphicsLayerChromium set the contentsLayerPurpose, but this doesn't appear to be used for anything.  I think we should just fold them together into one function that accepts any sort of PlatformLayer.

[Tim Horton]

Taking this.

[Tim Horton]

Created attachment 215943 [details]
patch

[Tim Horton]

It's possible we should get rid of ContentsLayerPurpose entirely, since it's only used to know if we need to dirty an added canvas layer (and know that some other way) or maybe we should use it more, and add it to our layer logging!

[EFL EWS Bot]

Comment on attachment 215943 [details]
patch

Attachment 215943 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/19648819

[EFL EWS Bot]

Comment on attachment 215943 [details]
patch

Attachment 215943 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/19648826

[Tim Horton]

Oh dear, somehow I missed CoordinatedGraphicsLayer::setContentsToCanvas and that might make this entirely infeasible.

[Noam Rosenthal]

(In reply to comment #6)
> Oh dear, somehow I missed CoordinatedGraphicsLayer::setContentsToCanvas and that might make this entirely infeasible.

I think it's fine, CoordinatedGraphicsLayer an query the platform layer to see if it needs to go via that special code path.

[Noam Rosenthal]

(In reply to comment #7)
> (In reply to comment #6)
> > Oh dear, somehow I missed CoordinatedGraphicsLayer::setContentsToCanvas and that might make this entirely infeasible.
> 
> I think it's fine, CoordinatedGraphicsLayer an query the platform layer to see if it needs to go via that special code path.

(I can probably try to do that tomorrow)

[kov's GTK+ EWS bot]

Comment on attachment 215943 [details]
patch

Attachment 215943 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/19648834

[Philippe Normand]

What's up with this patch? Any reason why it didn't land?

[Byungseon(Sun) Shin]

Created attachment 236712 [details]
Patch

[WebKit Commit Bot]

Attachment 236712 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byungseon(Sun) Shin]

Patch set #2 requires https://bugs.webkit.org/show_bug.cgi?id=136017 .

[Byungseon(Sun) Shin]

Created attachment 236713 [details]
Patch

[WebKit Commit Bot]

Attachment 236713 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byungseon(Sun) Shin]

Created attachment 236715 [details]
Patch

[WebKit Commit Bot]

Attachment 236715 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byungseon(Sun) Shin]

Created attachment 236716 [details]
Patch

[WebKit Commit Bot]

Attachment 236716 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byungseon(Sun) Shin]

Created attachment 236727 [details]
Patch

[WebKit Commit Bot]

Attachment 236727 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byungseon(Sun) Shin]

Created attachment 236729 [details]
Patch

[WebKit Commit Bot]

Attachment 236729 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Looks just refactoring patch. So, looks fine for coordinated graphics. Simon, could you take a look this patch again ?

[Martin Robinson]

Comment on attachment 236729 [details]
Patch

Based on Simon's earlier review, Gyuyoung's opinion, and a look through the texture mapper changes, I feel pretty comfortable r+ing this. We can roll out if there are problems.

[WebKit Commit Bot]

Comment on attachment 236729 [details]
Patch

Rejecting attachment 236729 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[WebKit Commit Bot]

Comment on attachment 236729 [details]
Patch

Rejecting attachment 236729 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 236729, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
atching file Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsScene.h
patching file Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h
patching file Source/WebCore/rendering/RenderLayerBacking.cpp
patching file Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Martin Robinson']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/5709631116541952

[Byungseon(Sun) Shin]

@Gyuyoung,  I will rebase the patch it again soon.

[Byungseon Shin]

Created attachment 237022 [details]
Patch

[WebKit Commit Bot]

Attachment 237022 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byungseon(Sun) Shin]

@Gyuyoung, I have rebased the patch. 
There was an WEBCORE_EXPORTS update Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h .

[Gyuyoung Kim]

(In reply to comment #31)
> @Gyuyoung, I have rebased the patch. 
> There was an WEBCORE_EXPORTS update Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h .

You already got r+ed from Martin. In that case, you can add his name to ChangeLog. Then, just request cq?. If so, committer's able to land your patch.

[Byungseon Shin]

Created attachment 237030 [details]
Patch

[WebKit Commit Bot]

Attachment 237030 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:235:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:92:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:93:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 3 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byungseon Shin]

@Gyuyoung, thanks for the advice.

[WebKit Commit Bot]

Comment on attachment 237030 [details]
Patch

Clearing flags on attachment: 237030

Committed r172889: <http://trac.webkit.org/changeset/172889>

[Philippe Normand]

I suppose this can be closed now.

[Daniel Bates]

(In reply to comment #36)
> (From update of attachment 237030 [details])
> Clearing flags on attachment: 237030
> 
> Committed r172889: <http://trac.webkit.org/changeset/172889>

This broke the iOS build. Committed build fix in <http://trac.webkit.org/changeset/172938>.