Summary: | [Qt] Repaint counter for accelerated compositing | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Noam Rosenthal <noam> | ||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | gustavo, noam, webkit.review.bot, xan.lopez | ||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Noam Rosenthal
2012-06-27 17:41:31 PDT
Created attachment 151401 [details]
Patch
Comment on attachment 151401 [details] Patch Attachment 151401 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13189010 Comment on attachment 151401 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151401&action=review > Source/WebCore/ChangeLog:17 > + Note that there is no integration with Preferences. That aproach was > + taken initially but revealed complex and overkill for such a > + debugging-only functionality. Thus, to disable it simply restart with > + the environment variable unset or set to some other value. Please explain what you've changed in TextureMapper[GL] > Source/WebKit2/ChangeLog:9 > + You should note somewhere that this is for WebKit2 only for now. > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:368 > + QFont font(QStringLiteral("Monospace"), pointSize, QFont::Bold); QString::fromUtf8 would work, seems like QStringLiteral doesn't build. > Source/WebKit2/UIProcess/texmap/LayerBackingStore.cpp:136 > + static bool showTileDebugVisuals = (qgetenv("QT_WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS") == "1"); > + if (!showTileDebugVisuals) > + continue; This should be in its own (static) function Created attachment 151483 [details]
Patch
Comment on attachment 151483 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151483&action=review Let's put the #if PLATFORM(QT) only when actually needed, this will help other ports (EFL?) do this when they so choose. > Source/WebCore/platform/graphics/texmap/TextureMapper.h:127 > +#if PLATFORM(QT) > + virtual void drawRepaintCounter(int value, int pointSize, const FloatPoint&, const TransformationMatrix& modelViewMatrix = TransformationMatrix()) = 0; > +#endif Doesn't need ifdef > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:364 > +#if PLATFORM(QT) > +void TextureMapperGL::drawRepaintCounter(int value, int pointSize, const FloatPoint& targetPoint, const TransformationMatrix& modelViewMatrix) #if should be inside, notImplemented otherwise. > Source/WebCore/platform/graphics/texmap/TextureMapperGL.h:55 > +#if PLATFORM(QT) > + virtual void drawRepaintCounter(int value, int pointSize, const FloatPoint&, const TransformationMatrix& modelViewMatrix = TransformationMatrix()) OVERRIDE; > +#endif Doesn't need ifdef > Source/WebCore/platform/graphics/texmap/TextureMapperImageBuffer.h:57 > +#if PLATFORM(QT) > + virtual void drawRepaintCounter(int value, int pointSize, const FloatPoint&, const TransformationMatrix& modelViewMatrix = TransformationMatrix()) OVERRIDE { }; > +#endif ditto > Source/WebKit2/UIProcess/texmap/LayerBackingStore.cpp:103 > +#if PLATFORM(QT) > +static bool shouldShowTileDebugVisuals() > +{ > + return (qgetenv("QT_WEBKIT_SHOW_COMPOSITING_DEBUG_VISUALS") == "1"); > +} You can put the ifdef inside, and return false if not in Qt. > Source/WebKit2/UIProcess/texmap/LayerBackingStore.cpp:146 > +#if PLATFORM(QT) > + static bool shouldDebug = shouldShowTileDebugVisuals(); > + if (!shouldDebug) > + continue; > + textureMapper->drawBorder(QColor(Qt::red), 2, tile->rect(), transform); > + textureMapper->drawRepaintCounter(static_cast<LayerBackingStoreTile*>(tile)->repaintCount(), 8, tilesToPaint[i]->rect().location(), transform); > +#endif Doesn't need the ifdef > Source/WebKit2/UIProcess/texmap/LayerBackingStore.h:40 > +#if PLATFORM(QT) > + , m_repaintCount(0) > +#endif Doesn't need the ifdef > Source/WebKit2/UIProcess/texmap/LayerBackingStore.h:48 > +#if PLATFORM(QT) > + inline void incrementRepaintCount() { ++m_repaintCount; } > + inline int repaintCount() const { return m_repaintCount; } > +#endif Doesn't need the ifdef > Source/WebKit2/UIProcess/texmap/LayerBackingStore.h:60 > +#if PLATFORM(QT) > + int m_repaintCount; > +#endif Doesn't need the ifdef Created attachment 151494 [details]
Patch
Comment on attachment 151494 [details]
Patch
LGTM
Comment on attachment 151494 [details] Patch Attachment 151494 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13207041 Created attachment 151509 [details]
Fix GTK build
Comment on attachment 151509 [details] Fix GTK build Clearing flags on attachment: 151509 Committed r122275: <http://trac.webkit.org/changeset/122275> All reviewed patches have been landed. Closing bug. |