cc: handling debug settings in new LayerTreeDebugState structure

cc/layer_tree_host.cc

Index: cc/layer_tree_host.cc
diff --git a/cc/layer_tree_host.cc b/cc/layer_tree_host.cc
index b423eb7b8c726139ea070a85a7e5c2693e728e18..670a9ce9d7f100ede571e7115a076db85dd37562 100644
--- a/cc/layer_tree_host.cc
+++ b/cc/layer_tree_host.cc
@@ -29,17 +29,31 @@
 using namespace std;
 
 namespace {
+
 static int numLayerTreeInstances;
+
+// merge blocks of memory - ORs num bytes from source into destination
+void* memmrg(void* destination, const void* source, size_t num)
+{
+    char* dstPtr = (char*)destination;
+    char* srcPtr = (char*)source;
+
+    for (size_t i = 0; i < num; i++)
+        dstPtr[i] |= srcPtr[i];
+
+    return destination;
+}
+
 }
 
 namespace cc {
 
 bool LayerTreeHost::s_needsFilterContext = false;
 
-LayerTreeSettings::LayerTreeSettings()
-    : acceleratePainting(false)
-    , showDebugBorders(false)
+LayerTreeDebugState::LayerTreeDebugState()
+    : showFPSCounter(false)
     , showPlatformLayerTree(false)
+    , showDebugBorders(false)
     , showPaintRects(false)
     , showPropertyChangedRects(false)
     , showSurfaceDamageRects(false)
@@ -47,6 +61,37 @@ LayerTreeSettings::LayerTreeSettings()
     , showReplicaScreenSpaceRects(false)
     , showOccludingRects(false)
     , showNonOccludingRects(false)
+{
+}
+
+LayerTreeDebugState::~LayerTreeDebugState()
+{
+}
+
+bool LayerTreeDebugState::showHudInfo() const
+{
+    return showFPSCounter || showPlatformLayerTree || showHudRects();
+}
+
+bool LayerTreeDebugState::showHudRects() const
+{
+    return showPaintRects || showPropertyChangedRects || showSurfaceDamageRects || showScreenSpaceRects || showReplicaScreenSpaceRects || showOccludingRects || showNonOccludingRects;
+}
+
+bool LayerTreeDebugState::equal(const LayerTreeDebugState& a, const LayerTreeDebugState& b)
+{
+    return memcmp(&a, &b, sizeof(LayerTreeDebugState)) == 0;

[danakj, 2012/11/27 20:23:15]

this is fine.

+}
+
+LayerTreeDebugState LayerTreeDebugState::merge(const LayerTreeDebugState& a, const LayerTreeDebugState& b)

[danakj, 2012/11/27 20:23:15]

better names than "a" and "b" please. initialState and requestedEnabledState?

[egraether, 2012/11/27 22:10:47]

this method just ORs the states and returns a new one, so there is no real
meaning to the parameters. I think putting something could be misleading. same
applies for the equal method.

+{
+    LayerTreeDebugState r(a);
+    memmrg(&r, &b, sizeof(LayerTreeDebugState));

[danakj, 2012/11/27 20:23:15]

i liked just doing the | for each flag before better.

[egraether, 2012/11/27 22:10:47]

that was just a little experiment, because I don't really like that big block of
| statements. I then realized that this wouldn't work to well as soon as you add
members that are numbers or anything.

[danakj, 2012/11/27 22:15:00]

merge(a, b) is super not clear what is happening though.

We should use names for the variables that show their intended use.

equals(a,b) doesn't make me wonder about what the inside of the function is
doing and what the purpose of the two variables is.

+    return r;
+}
+
+LayerTreeSettings::LayerTreeSettings()
+    : acceleratePainting(false)
     , renderVSyncEnabled(true)
     , perTilePaintingEnabled(false)
     , partialSwapEnabled(false)
@@ -61,15 +106,16 @@ LayerTreeSettings::LayerTreeSettings()
     , minimumOcclusionTrackingSize(gfx::Size(160, 160))
 {
     // TODO(danakj): Move this to chromium when we don't go through the WebKit API anymore.
-    showPropertyChangedRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowPropertyChangedRects);
-    showSurfaceDamageRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowSurfaceDamageRects);
-    showScreenSpaceRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowScreenSpaceRects);
-    showReplicaScreenSpaceRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowReplicaScreenSpaceRects);
-    showOccludingRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowOccludingRects);
-    showNonOccludingRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowNonOccludingRects);
     partialSwapEnabled = CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnablePartialSwap);
     backgroundColorInsteadOfCheckerboard = CommandLine::ForCurrentProcess()->HasSwitch(switches::kBackgroundColorInsteadOfCheckerboard);
     showOverdrawInTracing = CommandLine::ForCurrentProcess()->HasSwitch(switches::kTraceOverdraw);
+
+    initialDebugState.showPropertyChangedRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowPropertyChangedRects);
+    initialDebugState.showSurfaceDamageRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowSurfaceDamageRects);
+    initialDebugState.showScreenSpaceRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowScreenSpaceRects);
+    initialDebugState.showReplicaScreenSpaceRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowReplicaScreenSpaceRects);
+    initialDebugState.showOccludingRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowOccludingRects);
+    initialDebugState.showNonOccludingRects = CommandLine::ForCurrentProcess()->HasSwitch(cc::switches::kShowNonOccludingRects);
 }
 
 LayerTreeSettings::~LayerTreeSettings()
@@ -118,6 +164,7 @@ LayerTreeHost::LayerTreeHost(LayerTreeHostClient* client, const LayerTreeSetting
     , m_numTimesRecreateShouldFail(0)
     , m_numFailedRecreateAttempts(0)
     , m_settings(settings)
+    , m_debugState(settings.initialDebugState)
     , m_deviceScaleFactor(1)
     , m_visible(true)
     , m_pageScaleFactor(1)
@@ -288,6 +335,7 @@ void LayerTreeHost::finishCommitOnImplThread(LayerTreeHostImpl* hostImpl)
     hostImpl->setPageScaleFactorAndLimits(m_pageScaleFactor, m_minPageScaleFactor, m_maxPageScaleFactor);
     hostImpl->setBackgroundColor(m_backgroundColor);
     hostImpl->setHasTransparentBackground(m_hasTransparentBackground);
+    hostImpl->setDebugState(m_debugState);
 
     m_commitNumber++;
 }
@@ -298,12 +346,6 @@ void LayerTreeHost::createHUDLayerIfNeeded()
         m_hudLayer = HeadsUpDisplayLayer::create();
 }
 
-void LayerTreeHost::setShowFPSCounter(bool show)
-{
-    createHUDLayerIfNeeded();
-    m_hudLayer->setShowFPSCounter(show);
-}
-
 void LayerTreeHost::setFontAtlas(scoped_ptr<FontAtlas> fontAtlas)
 {
     createHUDLayerIfNeeded();
@@ -314,7 +356,7 @@ void LayerTreeHost::willCommit()
 {
     m_client->willCommit();
 
-    if (m_settings.showDebugInfo())
+    if (m_debugState.showHudInfo())
         createHUDLayerIfNeeded();
 
     if (m_rootLayer && m_hudLayer && !m_hudLayer->parent())
@@ -441,6 +483,14 @@ void LayerTreeHost::setRootLayer(scoped_refptr<Layer> rootLayer)
     setNeedsCommit();
 }
 
+void LayerTreeHost::setDebugState(const LayerTreeDebugState& debugState)
+{
+    if (!LayerTreeDebugState::equal(m_debugState, debugState)) {

[danakj, 2012/11/27 20:23:15]

I think you want to merge, then compare to m_debugState. Then
assign/setNeedsCommit if anything changed.

[egraether, 2012/11/27 22:10:47]

Yeah, that sounds more stable. I just tried to be efficient.

+        m_debugState = LayerTreeDebugState::merge(m_settings.initialDebugState, debugState);
+        setNeedsCommit();
+    }
+}
+
 void LayerTreeHost::setViewportSize(const gfx::Size& layoutViewportSize, const gfx::Size& deviceViewportSize)
 {
     if (layoutViewportSize == m_layoutViewportSize && deviceViewportSize == m_deviceViewportSize)