Bug 109661

Summary: Implement CoordinatedSurface for Threaded Coordinated Graphics
Product: WebKit Reporter: Jae Hyun Park <jaepark>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, commit-queue, eflews.bot, gyuyoung.kim, gyuyoung.kim, luiz, noam, rakuco, webkit-ews, webkit.review.bot, zeno
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 109659, 108899    
Bug Blocks: 102994, 117227    
Attachments:
Description Flags
Patch
webkit-ews: commit-queue-
Patch
none
Patch
none
Patch none

Description Jae Hyun Park 2013-02-12 21:34:09 PST 
To implement Threaded Coordinated Graphics, we need to implement CoordinatedSurface of WebKit1.

Currently, Coordinated Graphics is using WebCoordinatedSurface as CoordinatedSurface, which resides in Source/WebKit2.
WebCoordinatedSurface is implemented with some WebKit2 specific code, such as CoreIPC. So, we cannot reuse WebCoordinatedSurface in Threaded Coordinated Graphics. 
Therefore, WebKit1 CoordinatedSurface implementation with similar behavior is needed to implement Threaded Coordinated Graphics.
Comment 1 Jae Hyun Park 2013-02-12 21:50:45 PST 
Created attachment 188009 [details]
Patch
Comment 2 Jae Hyun Park 2013-02-12 21:52:21 PST 
(In reply to comment #1)
> Created an attachment (id=188009) [details]
> Patch

Sorry for the terrible naming. I couldn't come up with anything. Any suggestions on the naming?

This patch is the general idea for how WebKit1 CoordinatedSurface would be implemented.
Comment 3 Early Warning System Bot 2013-02-12 21:59:32 PST 
Comment on attachment 188009 [details]
Patch

Attachment 188009 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16440635
Comment 4 EFL EWS Bot 2013-02-12 22:00:01 PST 
Comment on attachment 188009 [details]
Patch

Attachment 188009 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16439647
Comment 5 Early Warning System Bot 2013-02-12 22:01:33 PST 
Comment on attachment 188009 [details]
Patch

Attachment 188009 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16483600
Comment 6 EFL EWS Bot 2013-04-23 21:46:01 PDT 
Comment on attachment 188009 [details]
Patch

Attachment 188009 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/219001
Comment 7 Jae Hyun Park 2013-06-05 03:24:49 PDT 
Created attachment 203785 [details]
Patch

Rebased to upstream.
Comment 8 EFL EWS Bot 2013-06-05 04:27:52 PDT 
Comment on attachment 203785 [details]
Patch

Attachment 203785 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/701475
Comment 9 Early Warning System Bot 2013-06-05 04:29:40 PDT 
Comment on attachment 203785 [details]
Patch

Attachment 203785 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/763265
Comment 10 Early Warning System Bot 2013-06-05 04:31:26 PDT 
Comment on attachment 203785 [details]
Patch

Attachment 203785 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/662549
Comment 11 EFL EWS Bot 2013-06-05 04:35:07 PDT 
Comment on attachment 203785 [details]
Patch

Attachment 203785 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/769090
Comment 12 Jae Hyun Park 2013-07-01 18:31:08 PDT 
Created attachment 205848 [details]
Patch
Comment 13 Noam Rosenthal 2013-07-01 21:36:44 PDT 
Comment on attachment 205848 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205848&action=review

> Source/WebCore/platform/graphics/texmap/coordinated/ThreadSafeCoordinatedSurface.cpp:58
> +    GraphicsContext* context = beginPaint(rect);
> +    client->paintToSurfaceContext(context);
> +    endPaint();

It would be better if this was all in one function. Otherwise it may look like beginPaint and endPaint can be called outside the context of paintToSurface.

> Source/WebCore/platform/graphics/texmap/coordinated/ThreadSafeCoordinatedSurface.cpp:77
> +#if USE(TEXTURE_MAPPER)

I think we can remove these in coordinated-graphics code.
Comment 14 Jae Hyun Park 2013-07-01 22:06:20 PDT 
Created attachment 205863 [details]
Patch
Comment 15 Jae Hyun Park 2013-07-01 22:10:45 PDT 
(In reply to comment #13)
> (From update of attachment 205848 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205848&action=review
> 
> > Source/WebCore/platform/graphics/texmap/coordinated/ThreadSafeCoordinatedSurface.cpp:58
> > +    GraphicsContext* context = beginPaint(rect);
> > +    client->paintToSurfaceContext(context);
> > +    endPaint();
> 
> It would be better if this was all in one function. Otherwise it may look like beginPaint and endPaint can be called outside the context of paintToSurface.

I agree. Removed beginPaint and endPaint.
> 
> > Source/WebCore/platform/graphics/texmap/coordinated/ThreadSafeCoordinatedSurface.cpp:77
> > +#if USE(TEXTURE_MAPPER)
> 
> I think we can remove these in coordinated-graphics code.
Fixed.


Thanks for the review!
Comment 16 WebKit Commit Bot 2013-07-02 12:17:57 PDT 
Comment on attachment 205863 [details]
Patch

Clearing flags on attachment: 205863

Committed r152307: <http://trac.webkit.org/changeset/152307>
Comment 17 WebKit Commit Bot 2013-07-02 12:18:01 PDT 
All reviewed patches have been landed.  Closing bug.