Bug 107224 - Coordinated Graphics: LayerTreeRenderer manages the surface of UpdateAtlas.
Summary: Coordinated Graphics: LayerTreeRenderer manages the surface of UpdateAtlas.
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: Dongseong Hwang
URL:
Keywords:
Depends on:
Blocks: 103854
  Show dependency tree / graph
 
Reported: 2013-01-17 21:53 PST by Dongseong Hwang
Modified: 2013-01-30 15:01 PST (History)
4 users (show)

See Also:

Attachments
Patch (8.70 KB, patch)
2013-01-17 21:54 PST, Dongseong Hwang 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2013-01-17 21:53:27 PST 
Currently, CoordinatedLayerTreeHostProxy manages the surface of UpdateAtlas, but
all other resources are managed by LayerTreeRenderer. This patch matches the
surface of UpdateAtlas to other resources.
Comment 1 Dongseong Hwang 2013-01-17 21:54:51 PST 
Created attachment 183370 [details]
Patch
Comment 2 Dongseong Hwang 2013-01-17 21:57:12 PST 
CoordinatedLayerTreeHostProxy::m_surface is obstacle that TextureMapperScene handles all resources, so I move m_surface from CoordinatedLayerTreeHostProxy to LayerTreeRenderer.
Comment 3 Dongseong Hwang 2013-01-24 21:31:59 PST 
noam: ping
Comment 4 Dongseong Hwang 2013-01-27 22:05:20 PST 
Could noam review please?
Comment 5 Noam Rosenthal 2013-01-27 23:02:44 PST 
Comment on attachment 183370 [details]
Patch

... why is this needed?
I think it makes sense for the web process to manage the lifestyle of update atlases.
Comment 6 Dongseong Hwang 2013-01-27 23:14:27 PST 
(In reply to comment #5)
> (From update of attachment 183370 [details])
> ... why is this needed?
> I think it makes sense for the web process to manage the lifestyle of update atlases.

web process still manage the lifecycle.
It just moves surface map from CoordinatedLayerTreeHostProxy to LayerTreeRenderer, because other resources (e.g. ImageBacking, Canvas Surface, CoordinatedTiledBackingStore, etc.) belong to LayerTreeRenderer.

On the other hands, I don't want CoordinatedLayerTreeHostProxy to have this kind of code because we will remain only enqueueCoordinatedOperation and commitCoordinatedCoperations in CoordinatedLayerTreeHostProxy.
Comment 7 Noam Rosenthal 2013-01-27 23:23:01 PST 
Comment on attachment 183370 [details]
Patch

LGTM
Comment 8 Noam Rosenthal 2013-01-27 23:23:22 PST 
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 183370 [details] [details])
> > ... why is this needed?
> > I think it makes sense for the web process to manage the lifestyle of update atlases.
> 
> web process still manage the lifecycle.
> It just moves surface map from CoordinatedLayerTreeHostProxy to LayerTreeRenderer, because other resources (e.g. ImageBacking, Canvas Surface, CoordinatedTiledBackingStore, etc.) belong to LayerTreeRenderer.
> 
> On the other hands, I don't want CoordinatedLayerTreeHostProxy to have this kind of code because we will remain only enqueueCoordinatedOperation and commitCoordinatedCoperations in CoordinatedLayerTreeHostProxy.

Right, I was misunderstanding the patch (haven't had coffee yet...)
Comment 9 Dongseong Hwang 2013-01-27 23:28:17 PST 
(In reply to comment #8)
> Right, I was misunderstanding the patch (haven't had coffee yet...)

hehe :)
Comment 10 Dongseong Hwang 2013-01-29 23:45:54 PST 
(In reply to comment #7)
> (From update of attachment 183370 [details])
> LGTM

benjaminp, could you take a look?
Comment 11 Benjamin Poulain 2013-01-30 14:46:55 PST 
Comment on attachment 183370 [details]
Patch

I am okay with this and Noam reviewed.
Comment 12 WebKit Review Bot 2013-01-30 15:01:52 PST 
Comment on attachment 183370 [details]
Patch

Clearing flags on attachment: 183370

Committed r141325: <http://trac.webkit.org/changeset/141325>
Comment 13 WebKit Review Bot 2013-01-30 15:01:57 PST 
All reviewed patches have been landed.  Closing bug.