[graphite] Search for layer from last RenderStep, insert remaining steps in reverse order Searching from the last RenderStep and inserting in reverse order allows the directional template parameter to searchBinding to be removed (less code size). Having the last step be the pilot draw for multi-step renderered draws is also advantageous because that is typically the shading step, which will have more constrained binding requirements. The remaining steps that are usually stencil-only will then match in bulk in the found layer. This also sets up the BindingList linked-list on Layer to be organized into two sections, a first "half" for non-shading draws (which can include the stencil steps of a shaded renderer), and then a second "half" for shading draws. In later CLs this will be used to end iterating over the Layer's lists when possible. Bug: 419535595 Change-Id: I04122548e0d68890c5e7d702003a3f5d82a8609e Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1283277 Reviewed-by: Thomas Smith <thomsmit@google.com> Commit-Queue: Thomas Smith <thomsmit@google.com>
diff --git a/src/gpu/graphite/DrawListLayer.cpp b/src/gpu/graphite/DrawListLayer.cpp index cff730b..47b0ab9 100644 --- a/src/gpu/graphite/DrawListLayer.cpp +++ b/src/gpu/graphite/DrawListLayer.cpp
@@ -68,8 +68,8 @@ // it drew into. targetLayer = stop.fLayer ? stop.fLayer : fLayers.head(); if (targetLayer) { - targetMatch = targetLayer->searchBinding</*kForwards=*/false>( - key, stop.fList, !fStorageBufferSupport); + targetMatch = targetLayer->searchBinding( + key, /*startList=*/nullptr, !fStorageBufferSupport); } } else { current = fLayers.tail(); @@ -236,10 +236,11 @@ SkASSERT(start.fLayer); SkASSERT(start.fList); BindingList* targetMatch = nullptr; - if (start.fList->fNext) { - targetMatch = start.fLayer->searchBinding</*kForwards=*/true>( - key, start.fList, !fStorageBufferSupport); - } + // Search through preceding bindings (exclusive) for a match for the new step. + if (start.fList->fPrev) { + targetMatch = start.fLayer->searchBinding( + key, start.fList->fPrev, !fStorageBufferSupport); + } // else there are no preceding bindings so we know we have to add a new one if (!targetMatch) { targetMatch = start.fLayer->addNewBinding( @@ -301,7 +302,7 @@ Insertion stepInsertion = {nullptr, nullptr}; fRenderStepCount += renderer->numRenderSteps(); bool canForwardMerge = renderer->numRenderSteps() == 1; - for (int stepIndex = 0; stepIndex < renderer->numRenderSteps(); ++stepIndex) { + for (int stepIndex = renderer->numRenderSteps() - 1; stepIndex >= 0; --stepIndex) { const RenderStep* const step = renderer->steps()[stepIndex]; gatherer->markOffsetAndAlign(step->performsShading(), step->uniformAlignment()); @@ -324,7 +325,6 @@ // Invalid ID implies depth only draw bool isDepthOnly = paintID == UniquePaintParamsID::Invalid(); - bool stepDependsOnDst = isDepthOnly || (stepIndex == 0 && dependsOnDst); LayerKey layerKey{pipelineIndex, textureBindingIndex, uniformIndex}; if (!stepInsertion) { @@ -332,7 +332,7 @@ stepIndex, rendererIsStencil, isDepthOnly, - stepDependsOnDst, + dependsOnDst || isDepthOnly, requiresBarrier, step, uniformIndex, @@ -353,8 +353,7 @@ SkASSERT(stepInsertion); stepInsertion.fList->addDraw(fStorage.make<Draw>(drawParams, uniformIndex), - /*backToFront=*/dependsOnDst || - stepInsertion.fList == latestInsertion.fList); + /*backToFront=*/dependsOnDst); gatherer->rewindForRenderStep(); }
diff --git a/src/gpu/graphite/DrawListTypes.h b/src/gpu/graphite/DrawListTypes.h index 40bec7f..cbde01d 100644 --- a/src/gpu/graphite/DrawListTypes.h +++ b/src/gpu/graphite/DrawListTypes.h
@@ -111,21 +111,18 @@ SkTInternalLList<BindingList> fBindings; SK_DECLARE_INTERNAL_LLIST_INTERFACE(Layer); - template <bool kForwards> - BindingList* searchBinding(const LayerKey& key, BindingList* startList, bool matchUniform) { - BindingList* list; - BindingList* end; - - if constexpr (kForwards) { - list = startList ? startList->fNext : fBindings.head(); - end = nullptr; - } else { - list = fBindings.tail(); - end = startList ? startList->fPrev : nullptr; + // Performs no bounds checks, so can only be used when checks have already confirmed the Layer + // is valid for adding a new draw into. This searches backwards from `startList` (inclusive) or + // the tail BindingList if null. + SK_ALWAYS_INLINE BindingList* searchBinding(const LayerKey& key, + BindingList* startList, + bool matchUniform) { + if (!startList) { + startList = fBindings.tail(); } // Advancement is evaluated at compile time - for (; list != end; list = kForwards ? list->fNext : list->fPrev) { + for (BindingList* list = startList; list != nullptr; list = list->fPrev) { if (list->fKey.isEqual(key, matchUniform)) { return list; } @@ -217,23 +214,41 @@ SK_ALWAYS_INLINE BindingList* addNewBinding(bool isDepthOnly, SkArenaAllocWithReset* alloc, - BindingList* parentList, + BindingList* insertBefore, const LayerKey& key, const RenderStep* step) { + SkASSERT(!insertBefore || fBindings.isInList(insertBefore)); + fListOrder = fListOrder.next(); BindingList* list = alloc->make<BindingList>(fListOrder, isDepthOnly); list->fKey = key; list->fStep = const_cast<RenderStep*>(step); list->fBounds = Rect::InfiniteInverted(); - if (isDepthOnly) { - if (parentList) { - fBindings.addAfter(list, parentList); - } else { - fBindings.addToHead(list); - } - } else { + // We need to insert the new list in the right place to keep fBindings organized with all + // non-shading layers before shading layers, while also ensuring that the new `list` comes + // before `insertBefore` (when non-null). + if (insertBefore && isDepthOnly == insertBefore->fIsDepthOnly) { + // Since both keys' shading state matches, putting the new list right in front of + // `insertBefore` will not split the two sections (regardless of whether it was in the + // shading or non-shading section). + fBindings.addBefore(list, insertBefore); + } else if (!isDepthOnly) { + // Since a new shading binding can only be inserted before other shading bindings, + // the only way to get to this branch is to not have an insertBefore target. As such, + // the simplest way to maintain keeping shading bindings in the latter half is to add + // to the tail. + SkASSERT(!insertBefore); fBindings.addToTail(list); + } else { + // A non-shading draw can have an `insertBefore` target that is a shading binding (e.g. + // where the final shading step was inserted in the layer). In that case, addBefore() + // would possibly split the shading bindings section of `fBindings`. Adding it to the + // head of the bindings' list preserves the guarantee that all non-shading bindings are + // at the start and satisfies adding it before the `insertBefore` (if it were non-null). + SkASSERT(isDepthOnly); + SkASSERT(!insertBefore || !insertBefore->fIsDepthOnly); + fBindings.addToHead(list); } return list;