Bug 109658

Summary:

Unify GraphicsLayer::setContentsToMedia and setContentsToCanvas

Product:

WebKit

Reporter:

Tien-Ren Chen <trchen>

Component:

Layout and Rendering

Assignee:

Tim Horton <thorton>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bunhere, cmarcelo, commit-queue, dbates, eflews.bot, esprehn+autocc, glenn, gtk-ews, gyuyoung.kim, jaepark, kondapallykalyan, luciano.wolf, luiz, mark.lam, mrobinson, noam, pnormand, rego+ews, ryuan.choi, sergio, simon.fraser, sun.shin, thorton, xan.lopez, xingri, zeno

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
patch	simon.fraser: review+, eflews.bot: commit-queue-
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	mrobinson: review+, commit-queue: commit-queue-
Patch	none
Patch	none

Tien-Ren Chen

Reported 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.

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(Sun) Shin	no flags	Details Formatted Diff Diff
Patch (34.94 KB, patch) 2014-08-16 06:19 PDT, Byungseon(Sun) Shin	no flags	Details Formatted Diff Diff
Patch (36.09 KB, patch) 2014-08-16 07:35 PDT, Byungseon(Sun) Shin	no flags	Details Formatted Diff Diff
Patch (36.72 KB, patch) 2014-08-16 08:14 PDT, Byungseon(Sun) Shin	no flags	Details Formatted Diff Diff
Patch (36.82 KB, patch) 2014-08-17 01:41 PDT, Byungseon(Sun) Shin	no flags	Details Formatted Diff Diff
Patch (39.60 KB, patch) 2014-08-17 03:28 PDT, Byungseon(Sun) 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
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Tim Horton

Comment 1 2013-11-04 11:45:13 PST

Taking this.

Tim Horton

Comment 2 2013-11-04 12:34:55 PST

Created attachment 215943 [details] patch

Tim Horton

Comment 3 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!

EFL EWS Bot

Comment 4 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

EFL EWS Bot

Comment 5 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

Tim Horton

Comment 6 2013-11-04 13:10:00 PST

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

Noam Rosenthal

Comment 7 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.

Noam Rosenthal

Comment 8 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)

kov's GTK+ EWS bot

Comment 9 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

Philippe Normand

Comment 10 2014-05-05 06:09:48 PDT

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

Byungseon(Sun) Shin

Comment 11 2014-08-16 06:04:56 PDT

Created attachment 236712 [details] Patch

WebKit Commit Bot

Comment 12 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.

Byungseon(Sun) Shin

Comment 13 2014-08-16 06:07:50 PDT

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

Byungseon(Sun) Shin

Comment 14 2014-08-16 06:19:55 PDT

Created attachment 236713 [details] Patch

WebKit Commit Bot

Comment 15 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.

Byungseon(Sun) Shin

Comment 16 2014-08-16 07:35:02 PDT

Created attachment 236715 [details] Patch

WebKit Commit Bot

Comment 17 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.

Byungseon(Sun) Shin

Comment 18 2014-08-16 08:14:41 PDT

Created attachment 236716 [details] Patch

WebKit Commit Bot

Comment 19 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.

Byungseon(Sun) Shin

Comment 20 2014-08-17 01:41:43 PDT

Created attachment 236727 [details] Patch

WebKit Commit Bot

Comment 21 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.

Byungseon(Sun) Shin

Comment 22 2014-08-17 03:28:13 PDT

Created attachment 236729 [details] Patch

WebKit Commit Bot

Comment 23 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.

Gyuyoung Kim

Comment 24 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 ?

Martin Robinson

Comment 25 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.

WebKit Commit Bot

Comment 26 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.

WebKit Commit Bot

Comment 27 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

Byungseon(Sun) Shin

Comment 28 2014-08-22 18:57:34 PDT

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

Byungseon Shin

Comment 29 2014-08-22 19:27:14 PDT

Created attachment 237022 [details] Patch

WebKit Commit Bot

Comment 30 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.

Byungseon(Sun) Shin

Comment 31 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 .

Gyuyoung Kim

Comment 32 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.

Byungseon Shin

Comment 33 2014-08-23 01:43:36 PDT

Created attachment 237030 [details] Patch

WebKit Commit Bot

Comment 34 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.

Byungseon Shin

Comment 35 2014-08-23 01:49:51 PDT

@Gyuyoung, thanks for the advice.

WebKit Commit Bot

Comment 36 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>

Philippe Normand

Comment 37 2014-08-25 02:18:17 PDT

I suppose this can be closed now.

Daniel Bates

Comment 38 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>.

Note You need to log in before you can comment on or make changes to this bug.