Bug 94129

Summary: Mark Skia and Compositor Contexts
Product: WebKit Reporter: Gregg Tavares <gman>
Component: PlatformAssignee: Gregg Tavares <gman>
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, enne, jamesr, senorblanco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Gregg Tavares 2012-08-15 11:21:03 PDT
Mark Skia and Compositor Contexts
Comment 1 Gregg Tavares 2012-08-15 11:22:29 PDT
Created attachment 158602 [details]
Patch
Comment 2 Gregg Tavares 2012-08-15 11:24:06 PDT
This CL just marks those contexts to aid in debugging.
Comment 3 James Robinson 2012-08-15 11:28:21 PDT
Comment on attachment 158602 [details]
Patch

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

Do you have to pop these markers ever?

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:694
> +    context3d->pushGroupMarkerEXT("CompositorContext");

This would be better in LayerRendererChromium::initialize() - all WGC3D stuff should be down there.
Comment 4 Gregg Tavares 2012-08-15 12:12:56 PDT
Created attachment 158614 [details]
Patch
Comment 5 Gregg Tavares 2012-08-15 12:14:17 PDT
moved marking the compositor context from CCLayerTreeHostImpl it to LayerRendererChromium
Comment 6 Gregg Tavares 2012-08-15 12:20:35 PDT
(In reply to comment #3)
> (From update of attachment 158602 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=158602&action=review
> 
> Do you have to pop these markers ever?

No (or yes). They basically are just for debugging so when the gpu process prints a message it will print the marker. pushGroupMarker is like namespace. insertEventMarker sets the current "marker" for the current group. So

pushGroupMaker("foo");
enable(1234); // invalid enum
insertEventMarker("orange");
pushGroupMaker("bar");
enable(1234); // invalid enum
insertEventMarker("hello");
enable(1234); // invalid enum
insertEventMarker("world");
enable(1234); // invalid enum
popGroupMarker();
enable(1234); // invalid enum

should print something like

.foo: glEnable INVALID_ENUM
.foo.orange: glEnable INVALID_ENUM
.foo.bar: glEnable INVALID_ENUM
.foo.bar.hello: glEnable INVALID_ENUM
.foo.bar.word: glEnable INVALID_ENUM
.foo.orange: glEnable INVALID_ENUM

It's safe to pop too many times.

These values can get passed down to the driver and used in perf tools but I'm not currently passing them down to the driver

> 
> > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:694
> > +    context3d->pushGroupMarkerEXT("CompositorContext");
> 
> This would be better in LayerRendererChromium::initialize() - all WGC3D stuff should be down there.
Comment 7 WebKit Review Bot 2012-08-15 13:51:59 PDT
Comment on attachment 158614 [details]
Patch

Clearing flags on attachment: 158614

Committed r125703: <http://trac.webkit.org/changeset/125703>
Comment 8 WebKit Review Bot 2012-08-15 13:52:02 PDT
All reviewed patches have been landed.  Closing bug.