Commit 23d63fc3 by Jamie Madill Committed by Commit Bot

Capture/Replay: Fix MEC Query capture.

In Manhattan the test generates a bunch of queries that it doesn't start until later frames. In ANGLE these queries will be stored as nullptr entries in the QueryMap. By default the QueryMap Iterator skips over nullptr entries. This manifested later as GL errors during replay. Fix this by adding a new Iterator type to ResourceMap that does not skip over nullptr values. Bug: angleproject:4489 Change-Id: If56b908fb233de0df0445f9ea19fc322f2c42976 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2107762 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCody Northrop <cnorthrop@google.com> Reviewed-by: 's avatarCourtney Goeltzenleuchter <courtneygo@google.com>
parent e54f7ed7
...@@ -848,7 +848,7 @@ void WriteCppReplayIndexFiles(bool compression, ...@@ -848,7 +848,7 @@ void WriteCppReplayIndexFiles(bool compression,
source << " GLuint returnedID;\n"; source << " GLuint returnedID;\n";
std::string captureNamespace = !captureLabel.empty() ? captureLabel + "::" : ""; std::string captureNamespace = !captureLabel.empty() ? captureLabel + "::" : "";
source << " memcpy(&returnedID, &" << captureNamespace source << " memcpy(&returnedID, &" << captureNamespace
<< "gReadBuffer[readBufferOffset], sizeof(GLuint));\n "; << "gReadBuffer[readBufferOffset], sizeof(GLuint));\n";
source << " (*resourceMap)[id] = returnedID;\n"; source << " (*resourceMap)[id] = returnedID;\n";
source << "}\n"; source << "}\n";
source << "\n"; source << "\n";
...@@ -1996,19 +1996,22 @@ void CaptureMidExecutionSetup(const gl::Context *context, ...@@ -1996,19 +1996,22 @@ void CaptureMidExecutionSetup(const gl::Context *context,
// TODO(http://anglebug.com/3662): ES 3.x objects. // TODO(http://anglebug.com/3662): ES 3.x objects.
// Create existing queries // Create existing queries. Note that queries may be genned and not yet started. In that
// case the queries will exist in the query map as nullptr entries.
const gl::QueryMap &queryMap = context->getQueriesForCapture(); const gl::QueryMap &queryMap = context->getQueriesForCapture();
for (const auto &queryIter : queryMap) for (gl::QueryMap::Iterator queryIter = queryMap.beginWithNull();
queryIter != queryMap.endWithNull(); ++queryIter)
{ {
ASSERT(queryIter.first); ASSERT(queryIter->first);
gl::QueryID queryID = {queryIter.first}; gl::QueryID queryID = {queryIter->first};
cap(CaptureGenQueries(replayState, true, 1, &queryID)); cap(CaptureGenQueries(replayState, true, 1, &queryID));
MaybeCaptureUpdateResourceIDs(setupCalls); MaybeCaptureUpdateResourceIDs(setupCalls);
if (queryIter.second) gl::Query *query = queryIter->second;
if (query)
{ {
gl::QueryType queryType = queryIter.second->getType(); gl::QueryType queryType = query->getType();
// Begin the query to generate the object // Begin the query to generate the object
cap(CaptureBeginQuery(replayState, true, queryType, queryID)); cap(CaptureBeginQuery(replayState, true, queryType, queryID));
......
...@@ -62,13 +62,15 @@ class ResourceMap final : angle::NonCopyable ...@@ -62,13 +62,15 @@ class ResourceMap final : angle::NonCopyable
friend class ResourceMap; friend class ResourceMap;
Iterator(const ResourceMap &origin, Iterator(const ResourceMap &origin,
GLuint flatIndex, GLuint flatIndex,
typename HashMap::const_iterator hashIndex); typename HashMap::const_iterator hashIndex,
bool skipNulls);
void updateValue(); void updateValue();
const ResourceMap &mOrigin; const ResourceMap &mOrigin;
GLuint mFlatIndex; GLuint mFlatIndex;
typename HashMap::const_iterator mHashIndex; typename HashMap::const_iterator mHashIndex;
IndexAndResource mValue; IndexAndResource mValue;
bool mSkipNulls;
}; };
// null values represent reserved handles. // null values represent reserved handles.
...@@ -76,13 +78,16 @@ class ResourceMap final : angle::NonCopyable ...@@ -76,13 +78,16 @@ class ResourceMap final : angle::NonCopyable
Iterator end() const; Iterator end() const;
Iterator find(IDType handle) const; Iterator find(IDType handle) const;
Iterator beginWithNull() const;
Iterator endWithNull() const;
// Not a constant-time operation, should only be used for verification. // Not a constant-time operation, should only be used for verification.
bool empty() const; bool empty() const;
private: private:
friend class Iterator; friend class Iterator;
GLuint nextNonNullResource(size_t flatIndex) const; GLuint nextResource(size_t flatIndex, bool skipNulls) const;
// constexpr methods cannot contain reinterpret_cast, so we need a static method. // constexpr methods cannot contain reinterpret_cast, so we need a static method.
static ResourceType *InvalidPointer(); static ResourceType *InvalidPointer();
...@@ -194,13 +199,27 @@ template <typename ResourceType, typename IDType> ...@@ -194,13 +199,27 @@ template <typename ResourceType, typename IDType>
typename ResourceMap<ResourceType, IDType>::Iterator ResourceMap<ResourceType, IDType>::begin() typename ResourceMap<ResourceType, IDType>::Iterator ResourceMap<ResourceType, IDType>::begin()
const const
{ {
return Iterator(*this, nextNonNullResource(0), mHashedResources.begin()); return Iterator(*this, nextResource(0, true), mHashedResources.begin(), true);
} }
template <typename ResourceType, typename IDType> template <typename ResourceType, typename IDType>
typename ResourceMap<ResourceType, IDType>::Iterator ResourceMap<ResourceType, IDType>::end() const typename ResourceMap<ResourceType, IDType>::Iterator ResourceMap<ResourceType, IDType>::end() const
{ {
return Iterator(*this, static_cast<GLuint>(mFlatResourcesSize), mHashedResources.end()); return Iterator(*this, static_cast<GLuint>(mFlatResourcesSize), mHashedResources.end(), true);
}
template <typename ResourceType, typename IDType>
typename ResourceMap<ResourceType, IDType>::Iterator
ResourceMap<ResourceType, IDType>::beginWithNull() const
{
return Iterator(*this, nextResource(0, false), mHashedResources.begin(), false);
}
template <typename ResourceType, typename IDType>
typename ResourceMap<ResourceType, IDType>::Iterator
ResourceMap<ResourceType, IDType>::endWithNull() const
{
return Iterator(*this, static_cast<GLuint>(mFlatResourcesSize), mHashedResources.end(), false);
} }
template <typename ResourceType, typename IDType> template <typename ResourceType, typename IDType>
...@@ -234,11 +253,12 @@ void ResourceMap<ResourceType, IDType>::clear() ...@@ -234,11 +253,12 @@ void ResourceMap<ResourceType, IDType>::clear()
} }
template <typename ResourceType, typename IDType> template <typename ResourceType, typename IDType>
GLuint ResourceMap<ResourceType, IDType>::nextNonNullResource(size_t flatIndex) const GLuint ResourceMap<ResourceType, IDType>::nextResource(size_t flatIndex, bool skipNulls) const
{ {
for (size_t index = flatIndex; index < mFlatResourcesSize; index++) for (size_t index = flatIndex; index < mFlatResourcesSize; index++)
{ {
if (mFlatResources[index] != nullptr && mFlatResources[index] != InvalidPointer()) if ((mFlatResources[index] != nullptr || !skipNulls) &&
mFlatResources[index] != InvalidPointer())
{ {
return static_cast<GLuint>(index); return static_cast<GLuint>(index);
} }
...@@ -257,8 +277,9 @@ template <typename ResourceType, typename IDType> ...@@ -257,8 +277,9 @@ template <typename ResourceType, typename IDType>
ResourceMap<ResourceType, IDType>::Iterator::Iterator( ResourceMap<ResourceType, IDType>::Iterator::Iterator(
const ResourceMap &origin, const ResourceMap &origin,
GLuint flatIndex, GLuint flatIndex,
typename ResourceMap<ResourceType, IDType>::HashMap::const_iterator hashIndex) typename ResourceMap<ResourceType, IDType>::HashMap::const_iterator hashIndex,
: mOrigin(origin), mFlatIndex(flatIndex), mHashIndex(hashIndex) bool skipNulls)
: mOrigin(origin), mFlatIndex(flatIndex), mHashIndex(hashIndex), mSkipNulls(skipNulls)
{ {
updateValue(); updateValue();
} }
...@@ -281,7 +302,7 @@ operator++() ...@@ -281,7 +302,7 @@ operator++()
{ {
if (mFlatIndex < static_cast<GLuint>(mOrigin.mFlatResourcesSize)) if (mFlatIndex < static_cast<GLuint>(mOrigin.mFlatResourcesSize))
{ {
mFlatIndex = mOrigin.nextNonNullResource(mFlatIndex + 1); mFlatIndex = mOrigin.nextResource(mFlatIndex + 1, mSkipNulls);
} }
else else
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment