Summary: | [GTK] Add compilation options to enable/disable Accelerated Compositing and to choose texture mapper implementation. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nayan Kumar K <nayankk> | ||||||||||
Component: | WebKitGTK | Assignee: | Nayan Kumar K <nayankk> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | alex, mrobinson, noam | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 73440 | ||||||||||||
Bug Blocks: | 73319 | ||||||||||||
Attachments: |
|
Description
Nayan Kumar K
2011-11-30 09:35:41 PST
Created attachment 117246 [details]
Patch
Comment on attachment 117246 [details]
Patch
Now that the patches on which this bug depends are already in trunk, this patch should get applied without any conflicts.
Created attachment 117380 [details]
Compilation options to choose AC method to use
Patch rebased to latest of trunk
Comment on attachment 117380 [details]
Compilation options to choose AC method to use
I think that TEXTURE_MAPPER_CLUTTER might be a misleading name because the Clutter implementation of AC doesn't use TextureMapper.
Created attachment 117384 [details]
Option to choose AC method to use
Incorporated review comments
Comment on attachment 117384 [details] Option to choose AC method to use View in context: https://bugs.webkit.org/attachment.cgi?id=117384&action=review Looks good, just some suggestions. > Source/WebCore/GNUmakefile.am:568 > +if !USE_AC_CLUTTER I've rather used the positive condition in this case, I think it is safer, actually you can probably remove the previous if and just check if USE_AC_TEXTURE_MAPPER_OPENGL || USE_AC_TEXTURE_MAPPER_OPENGL, in future patches we could add the proper layout if there is another options in the if. > Source/WebCore/GNUmakefile.list.am:5078 > +if !USE_AC_CLUTTER Same here. > Source/WebCore/platform/graphics/GraphicsLayer.h:77 > +#if !USE(AC_CLUTTER) Same here > configure.ac:401 > +if test "$enable_webgl" = "yes" || test "$with_accelerated_compositing" = "clutter" || test "$with_accelerated_compositing" = "texmap-cairo" || test "$with_accelerated_compositing" == "texmap-opengl" ; then In case of clutter shouln't we check clutter library instead of GL? > configure.ac:1160 > +AM_CONDITIONAL([USE_AC_CLUTTER], [test "$with_accelerated_compositing" = "clutter"]) > +AM_CONDITIONAL([USE_AC_TEXTURE_MAPPER_OPENGL], [test "$with_accelerated_compositing" = "texmap-opengl"]) > +AM_CONDITIONAL([USE_AC_TEXTURE_MAPPER_CAIRO], [test "$with_accelerated_compositing" = "texmap-cairo"]) I think abbreviations are not used in the defines to improve readability, even if we have a huge name, not sure if we can use just a part of it. Created attachment 117431 [details]
Option to choose AC method to use
Incorporated review comments
Comment on attachment 117431 [details]
Option to choose AC method to use
LGTM
Comment on attachment 117431 [details] Option to choose AC method to use View in context: https://bugs.webkit.org/attachment.cgi?id=117431&action=review Just a couple minor changes to suggest. I will land this with my suggestions. > Source/WebCore/GNUmakefile.am:568 > +if USE_ACCELERATED_COMPOSITING_TEXTURE_MAPPER_CAIRO I think we should call these TEXTURE_MAPPER AND TEXTURE_MAPPER_GL to maintain consistency with Qt. > configure.ac:397 > +if test "$enable_webgl" = "yes" || test "$with_accelerated_compositing" = "texmap-cairo" || test "$with_accelerated_compositing" = "texmap-opengl" ; then There's an extra space before the second "test" here. > configure.ac:1156 > +AM_CONDITIONAL([USE_ACCELERATED_COMPOSITING], [test "$with_accelerated_compositing" = "texmap-cairo"]) > +AM_CONDITIONAL([USE_ACCELERATED_COMPOSITING], [test "$with_accelerated_compositing" = "texmap-opengl"]) > +AM_CONDITIONAL([USE_ACCELERATED_COMPOSITING_TEXTURE_MAPPER_CAIRO], [test "$with_accelerated_compositing" = "texmap-cairo"]) > +AM_CONDITIONAL([USE_ACCELERATED_COMPOSITING_TEXTURE_MAPPER_OPENGL], [test "$with_accelerated_compositing" = "texmap-opengl"]) Maybe just cairo, opengl and clutter. Committed r101764: <http://trac.webkit.org/changeset/101764> |