Bug 109658 - Unify GraphicsLayer::setContentsToMedia and setContentsToCanvas
Summary: Unify GraphicsLayer::setContentsToMedia and setContentsToCanvas
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-12 21:00 PST by Tien-Ren Chen
Modified: 2014-08-25 15:16 PDT (History)
26 users (show)

See Also:


Attachments
patch (20.36 KB, patch)
2013-11-04 12:34 PST, Tim Horton
simon.fraser: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (34.85 KB, patch)
2014-08-16 06:04 PDT, Byungseon Shin
no flags Details | Formatted Diff | Diff
Patch (34.94 KB, patch)
2014-08-16 06:19 PDT, Byungseon Shin
no flags Details | Formatted Diff | Diff
Patch (36.09 KB, patch)
2014-08-16 07:35 PDT, Byungseon Shin
no flags Details | Formatted Diff | Diff
Patch (36.72 KB, patch)
2014-08-16 08:14 PDT, Byungseon Shin
no flags Details | Formatted Diff | Diff
Patch (36.82 KB, patch)
2014-08-17 01:41 PDT, Byungseon Shin
no flags Details | Formatted Diff | Diff
Patch (39.60 KB, patch)
2014-08-17 03:28 PDT, Byungseon Shin
mrobinson: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch (40.60 KB, patch)
2014-08-22 19:27 PDT, Byungseon Shin
no flags Details | Formatted Diff | Diff
Patch (40.54 KB, patch)
2014-08-23 01:43 PDT, Byungseon Shin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tien-Ren Chen 2013-02-12 21:00:18 PST
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.
Comment 1 Tim Horton 2013-11-04 11:45:13 PST
Taking this.
Comment 2 Tim Horton 2013-11-04 12:34:55 PST
Created attachment 215943 [details]
patch
Comment 3 Tim Horton 2013-11-04 12:37:04 PST
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!
Comment 4 EFL EWS Bot 2013-11-04 12:50:21 PST
Comment on attachment 215943 [details]
patch

Attachment 215943 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/19648819
Comment 5 EFL EWS Bot 2013-11-04 13:07:15 PST
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
Comment 6 Tim Horton 2013-11-04 13:10:00 PST
Oh dear, somehow I missed CoordinatedGraphicsLayer::setContentsToCanvas and that might make this entirely infeasible.
Comment 7 Noam Rosenthal 2013-11-04 13:47:04 PST
(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.
Comment 8 Noam Rosenthal 2013-11-04 13:47:51 PST
(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)
Comment 9 kov's GTK+ EWS bot 2013-11-04 14:47:22 PST
Comment on attachment 215943 [details]
patch

Attachment 215943 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/19648834
Comment 10 Philippe Normand 2014-05-05 06:09:48 PDT
What's up with this patch? Any reason why it didn't land?
Comment 11 Byungseon Shin 2014-08-16 06:04:56 PDT
Created attachment 236712 [details]
Patch
Comment 12 WebKit Commit Bot 2014-08-16 06:06:40 PDT
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.
Comment 13 Byungseon Shin 2014-08-16 06:07:50 PDT
Patch set #2 requires https://bugs.webkit.org/show_bug.cgi?id=136017 .
Comment 14 Byungseon Shin 2014-08-16 06:19:55 PDT
Created attachment 236713 [details]
Patch
Comment 15 WebKit Commit Bot 2014-08-16 06:21:23 PDT
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.
Comment 16 Byungseon Shin 2014-08-16 07:35:02 PDT
Created attachment 236715 [details]
Patch
Comment 17 WebKit Commit Bot 2014-08-16 07:37:15 PDT
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.
Comment 18 Byungseon Shin 2014-08-16 08:14:41 PDT
Created attachment 236716 [details]
Patch
Comment 19 WebKit Commit Bot 2014-08-16 08:16:49 PDT
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.
Comment 20 Byungseon Shin 2014-08-17 01:41:43 PDT
Created attachment 236727 [details]
Patch
Comment 21 WebKit Commit Bot 2014-08-17 01:43:32 PDT
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.
Comment 22 Byungseon Shin 2014-08-17 03:28:13 PDT
Created attachment 236729 [details]
Patch
Comment 23 WebKit Commit Bot 2014-08-17 03:30:39 PDT
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.
Comment 24 Gyuyoung Kim 2014-08-21 19:17:08 PDT
Looks just refactoring patch. So, looks fine for coordinated graphics. Simon, could you take a look this patch again ?
Comment 25 Martin Robinson 2014-08-22 10:46:55 PDT
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.
Comment 26 WebKit Commit Bot 2014-08-22 18:43:47 PDT
Comment on attachment 236729 [details]
Patch

Rejecting attachment 236729 [details] from commit-queue.

sun.shin@lge.com 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.
Comment 27 WebKit Commit Bot 2014-08-22 18:48:33 PDT
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
Comment 28 Byungseon Shin 2014-08-22 18:57:34 PDT
@Gyuyoung,  I will rebase the patch it again soon.
Comment 29 Byungseon Shin 2014-08-22 19:27:14 PDT
Created attachment 237022 [details]
Patch
Comment 30 WebKit Commit Bot 2014-08-22 19:30:07 PDT
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.
Comment 31 Byungseon Shin 2014-08-22 20:16:13 PDT
@Gyuyoung, I have rebased the patch. 
There was an WEBCORE_EXPORTS update Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h .
Comment 32 Gyuyoung Kim 2014-08-22 20:52:31 PDT
(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.
Comment 33 Byungseon Shin 2014-08-23 01:43:36 PDT
Created attachment 237030 [details]
Patch
Comment 34 WebKit Commit Bot 2014-08-23 01:45:43 PDT
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.
Comment 35 Byungseon Shin 2014-08-23 01:49:51 PDT
@Gyuyoung, thanks for the advice.
Comment 36 WebKit Commit Bot 2014-08-23 02:24:56 PDT
Comment on attachment 237030 [details]
Patch

Clearing flags on attachment: 237030

Committed r172889: <http://trac.webkit.org/changeset/172889>
Comment 37 Philippe Normand 2014-08-25 02:18:17 PDT
I suppose this can be closed now.
Comment 38 Daniel Bates 2014-08-25 15:16:22 PDT
(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>.