From 30b02bc1ddfe1741c6e5cd5cd5c3bee0440b03e4 Mon Sep 17 00:00:00 2001
From: Benson Joeris <bjoeris@google.com>
Date: Mon, 8 Jul 2019 11:59:29 -0400
Subject: [PATCH] Refactor ComposeFrameRefsUnordered

Change-Id: I4d80b626c3c37a4c576024b3f8c2145dcb3bf108
---
 renderdoc/core/resource_manager.cpp | 52 +++++++++++++++++------------
 renderdoc/core/resource_manager.h   |  4 +++
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/renderdoc/core/resource_manager.cpp b/renderdoc/core/resource_manager.cpp
index fdbedcc8f..298afdc15 100644
--- a/renderdoc/core/resource_manager.cpp
+++ b/renderdoc/core/resource_manager.cpp
@@ -69,9 +69,6 @@ rdcstr DoStringise(const FrameRefType &el)
 
 FrameRefType ComposeFrameRefs(FrameRefType first, FrameRefType second)
 {
-  RDCASSERT(eFrameRef_Minimum <= first && first <= eFrameRef_Maximum);
-  RDCASSERT(eFrameRef_Minimum <= second && second <= eFrameRef_Maximum);
-
   switch(first)
   {
     case eFrameRef_None:
@@ -114,25 +111,17 @@ FrameRefType ComposeFrameRefs(FrameRefType first, FrameRefType second)
 
 FrameRefType ComposeFrameRefsUnordered(FrameRefType first, FrameRefType second)
 {
-  RDCASSERT(eFrameRef_Minimum <= first && first <= eFrameRef_Maximum);
-  RDCASSERT(eFrameRef_Minimum <= second && second <= eFrameRef_Maximum);
-
-  if(first == eFrameRef_Read &&
-     (second == eFrameRef_PartialWrite || second == eFrameRef_CompleteWrite))
-    // The resource is referenced both read and write/clear;
-    // We don't know whether the read or write/clear occurs first;
-    // if the write happens first, the final state would be Read or Clear;
-    // if the read happens first, the final state would be ReadBeforeWrite.
-    // We conservatively return ReadBeforeWrite, because this will force the
-    // resource to be reset before each frame when replaying.
+  if((IncludesRead(first) && IncludesWrite(second)) || (IncludesRead(second) && IncludesWrite(first)))
+    // There is a way to order these references so that the resource is read
+    // and then written. Since read-before-write is the worst case in terms of
+    // reset requirements, we conservatively assume this is the case.
     return eFrameRef_ReadBeforeWrite;
-
-  // In all other cases, we just return the more conservative reference type--
-  // i.e. the reference type with the strongest (re)initialization
-  // requirements for replay. Because larger values in the `FrameRefType` have
-  // stronger (re)initialization requirements, this is simply the maximum
-  // reference type; note that `first >= second` by the earlier swap.
-  return first;
+  else
+    // Otherwise either:
+    // - first and second are each Read or None, or
+    // - first and second are each CompleteWrite, PartialWrite or None.
+    // In either case, Compose(first,second) = Compose(second,first) = max(first,second).
+    return RDCMAX(first, second);
 }
 
 FrameRefType ComposeFrameRefsDisjoint(FrameRefType x, FrameRefType y)
@@ -140,6 +129,27 @@ FrameRefType ComposeFrameRefsDisjoint(FrameRefType x, FrameRefType y)
   return RDCMAX(x, y);
 }
 
+bool IncludesRead(FrameRefType refType)
+{
+  switch(refType)
+  {
+    case eFrameRef_Read:
+    case eFrameRef_ReadBeforeWrite: return true;
+    default: return false;
+  }
+}
+
+bool IncludesWrite(FrameRefType refType)
+{
+  switch(refType)
+  {
+    case eFrameRef_PartialWrite:
+    case eFrameRef_CompleteWrite:
+    case eFrameRef_ReadBeforeWrite: return true;
+    default: return false;
+  }
+}
+
 bool IsDirtyFrameRef(FrameRefType refType)
 {
   return (refType != eFrameRef_None && refType != eFrameRef_Read);
diff --git a/renderdoc/core/resource_manager.h b/renderdoc/core/resource_manager.h
index 3fa1028f1..cde82540f 100644
--- a/renderdoc/core/resource_manager.h
+++ b/renderdoc/core/resource_manager.h
@@ -94,6 +94,10 @@ enum FrameRefType
   eFrameRef_ReadBeforeWrite = 4,
 };
 
+bool IncludesRead(FrameRefType refType);
+
+bool IncludesWrite(FrameRefType refType);
+
 const FrameRefType eFrameRef_Minimum = eFrameRef_None;
 const FrameRefType eFrameRef_Maximum = eFrameRef_ReadBeforeWrite;