Change how render states are reference counted. Add unit test.

backpackcoder · Sep 16, 2015 · 000291f · 000291f
1 parent 3da95af
commit 000291f
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 13 deletions.
diff --git a/Source/Renderer/RenderState.js b/Source/Renderer/RenderState.js
@@ -272,7 +272,6 @@ define([
         //>>includeEnd('debug');
 
         this.id = 0;
-        this._referenceCount = 0;
         this._applyFunctions = [];
     };
 
@@ -402,8 +401,8 @@ define([
         var partialKey = JSON.stringify(renderState);
         var cachedState = renderStateCache[partialKey];
         if (defined(cachedState)) {
-            ++cachedState._referenceCount;
-            return cachedState;
+            ++cachedState.referenceCount;
+            return cachedState.state;
         }
 
         // Cache miss.  Fully define render state and try again.
@@ -413,45 +412,73 @@ define([
         if (!defined(cachedState)) {
             states.id = nextRenderStateId++;
 
-            cachedState = states;
+            cachedState = {
+                referenceCount : 0,
+                state : states
+            };
 
             // Cache full render state.  Multiple partially defined render states may map to this.
             renderStateCache[fullKey] = cachedState;
         }
 
-        ++cachedState._referenceCount;
+        ++cachedState.referenceCount;
 
         // Cache partial render state so we can skip validation on a cache hit for a partially defined render state
-        renderStateCache[partialKey] = cachedState;
+        renderStateCache[partialKey] = {
+            referenceCount : 1,
+            state : cachedState.state
+        };
 
-        return cachedState;
+        return cachedState.state;
     };
 
     /**
      * @private
      */
     RenderState.removeFromCache = function(renderState) {
+        var states = new RenderState(renderState);
+        var fullKey = JSON.stringify(states);
+        var fullCachedState = renderStateCache[fullKey];
+
         // decrement partial key reference count
         var partialKey = JSON.stringify(renderState);
         var cachedState = renderStateCache[partialKey];
         if (defined(cachedState)) {
-            --cachedState._referenceCount;
+            --cachedState.referenceCount;
 
-            if (cachedState._referenceCount === 0) {
+            if (cachedState.referenceCount === 0) {
                 // remove partial key
                 delete renderStateCache[partialKey];
+
+                // decrement full key reference count
+                if (defined(fullCachedState)) {
+                    --fullCachedState.referenceCount;
+                }
             }
         }
 
         // remove full key if reference count is zero
-        var states = new RenderState(renderState);
-        var fullKey = JSON.stringify(states);
-        cachedState = renderStateCache[fullKey];
-        if (defined(cachedState) && (cachedState._referenceCount === 0)) {
+        if (defined(fullCachedState) && (fullCachedState.referenceCount === 0)) {
             delete renderStateCache[fullKey];
         }
     };
 
+    /**
+     * This function is for testing purposes only.
+     * @private
+     */
+    RenderState.getCache = function() {
+        return renderStateCache;
+    };
+
+    /**
+     * This function is for testing purposes only.
+     * @private
+     */
+    RenderState.clearCache = function() {
+        renderStateCache = {};
+    };
+
     function enableOrDisable(gl, glEnum, enable) {
         if (enable) {
             gl.enable(glEnum);

diff --git a/Specs/Renderer/RenderStateSpec.js b/Specs/Renderer/RenderStateSpec.js
@@ -363,6 +363,52 @@ defineSuite([
         expect(rs4).not.toBe(rs);
     });
 
+    it('removes from render cache', function() {
+        RenderState.clearCache();
+        var cache = RenderState.getCache();
+
+        var rs = RenderState.fromCache();
+        var undefinedKey = JSON.stringify(undefined);
+        var fullKey = JSON.stringify(new RenderState());
+
+        expect(cache[fullKey].referenceCount).toEqual(1);
+        expect(cache[undefinedKey].referenceCount).toEqual(1);
+
+        var rs2 = RenderState.fromCache();
+
+        expect(cache[fullKey].referenceCount).toEqual(1);
+        expect(cache[undefinedKey].referenceCount).toEqual(2);
+
+        // rs3 is still the same state as rs and rs2, but with a partial definition
+        var param = {
+            depthTest : {
+                enabled : false,
+                func : WebGLConstants.LESS
+            }
+        };
+        var rs3 = RenderState.fromCache(param);
+        var paramKey = JSON.stringify(param);
+
+        expect(rs2).toBe(rs);
+        expect(rs3).toBe(rs);
+
+        expect(cache[fullKey].referenceCount).toEqual(2);
+        expect(cache[undefinedKey].referenceCount).toEqual(2);
+        expect(cache[paramKey].referenceCount).toEqual(1);
+
+        RenderState.removeFromCache(param);
+
+        expect(cache[fullKey].referenceCount).toEqual(1);
+        expect(cache[undefinedKey].referenceCount).toEqual(2);
+        expect(cache[paramKey]).not.toBeDefined();
+
+        RenderState.removeFromCache();
+        RenderState.removeFromCache();
+
+        expect(cache[undefinedKey]).not.toBeDefined();
+        expect(cache[fullKey]).not.toBeDefined();
+    });
+
     it('fails to create (frontFace)', function() {
         expect(function() {
             RenderState.fromCache({