Skip to content

Commit af3dd43

Browse files
csmartdalton86Skia Commit-Bot
authored andcommitted
Revert "Disable instanced rendering on Skylake"
This reverts commit 240ea4c. Reason for revert: Too many regressions from not having CCPR Original change's description: > Disable instanced rendering on Skylake > > We previously had a glFlush() workaround for instanced rendering on > Skylake. However, the non-instanced approach is often faster than > instanced + glFlush(). This CL just disables instanced rendering > altogether on Skylake instead. The chip is old enough now that this > seems like a reasonable solution. > > Bug: skia:8566 > Change-Id: Ib82a519d8186b463b72b20203fb69d078e757aa7 > Reviewed-on: https://skia-review.googlesource.com/c/172470 > Reviewed-by: Brian Salomon <bsalomon@google.com> > Commit-Queue: Chris Dalton <csmartdalton@google.com> TBR=bsalomon@google.com,csmartdalton@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: skia:8566 Change-Id: I947cdd0026b7fc31a4f75f5f416299e27dd6f56e Reviewed-on: https://skia-review.googlesource.com/c/173128 Reviewed-by: Chris Dalton <csmartdalton@google.com> Commit-Queue: Chris Dalton <csmartdalton@google.com>
1 parent a23de1d commit af3dd43

File tree

4 files changed

+33
-4
lines changed

4 files changed

+33
-4
lines changed

src/gpu/gl/GrGLCaps.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ GrGLCaps::GrGLCaps(const GrContextOptions& contextOptions,
6161
fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO = false;
6262
fUseDrawInsteadOfAllRenderTargetWrites = false;
6363
fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines = false;
64+
fRequiresFlushBetweenNonAndInstancedDraws = false;
6465
fDetachStencilFromMSAABuffersBeforeReadPixels = false;
6566
fClampMaxTextureLevelToOne = false;
6667
fProgramBinarySupport = false;
@@ -2468,13 +2469,10 @@ void GrGLCaps::applyDriverCorrectnessWorkarounds(const GrGLContextInfo& ctxInfo,
24682469
fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines = true;
24692470
}
24702471

2471-
// Intel Skylake instanced draws get corrupted if we mix them with normal ones. Adding a flush
2472-
// in between seems to resolve this, but it also tends to cause perf regressions. So we just
2473-
// disable instancing altogether on Skylake.
24742472
if (kIntelSkylake_GrGLRenderer == ctxInfo.renderer() ||
24752473
(kANGLE_GrGLRenderer == ctxInfo.renderer() &&
24762474
GrGLANGLERenderer::kSkylake == ctxInfo.angleRenderer())) {
2477-
fInstanceAttribSupport = false;
2475+
fRequiresFlushBetweenNonAndInstancedDraws = true;
24782476
}
24792477

24802478
// This was reproduced on a Pixel 1, but the unit test + config + options that exercise it are
@@ -2762,6 +2760,7 @@ void GrGLCaps::onApplyOptionsOverrides(const GrContextOptions& options) {
27622760
SkASSERT(!fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO);
27632761
SkASSERT(!fUseDrawInsteadOfAllRenderTargetWrites);
27642762
SkASSERT(!fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines);
2763+
SkASSERT(!fRequiresFlushBetweenNonAndInstancedDraws);
27652764
SkASSERT(!fDetachStencilFromMSAABuffersBeforeReadPixels);
27662765
}
27672766
if (GrContextOptions::Enable::kNo == options.fUseDrawInsteadOfGLClear) {

src/gpu/gl/GrGLCaps.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,12 @@ class GrGLCaps : public GrCaps {
385385
return fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines;
386386
}
387387

388+
// Intel Skylake instanced draws get corrupted if we mix them with normal ones. Adding a flush
389+
// in between seems to resolve this.
390+
bool requiresFlushBetweenNonAndInstancedDraws() const {
391+
return fRequiresFlushBetweenNonAndInstancedDraws;
392+
}
393+
388394
// Some Adreno drivers refuse to ReadPixels from an MSAA buffer that has stencil attached.
389395
bool detachStencilFromMSAABuffersBeforeReadPixels() const {
390396
return fDetachStencilFromMSAABuffersBeforeReadPixels;
@@ -529,6 +535,7 @@ class GrGLCaps : public GrCaps {
529535
bool fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO : 1;
530536
bool fUseDrawInsteadOfAllRenderTargetWrites : 1;
531537
bool fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines : 1;
538+
bool fRequiresFlushBetweenNonAndInstancedDraws : 1;
532539
bool fDetachStencilFromMSAABuffersBeforeReadPixels : 1;
533540
bool fClampMaxTextureLevelToOne : 1;
534541
int fMaxInstancesPerDrawWithoutCrashing;

src/gpu/gl/GrGLGpu.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,9 @@ void GrGLGpu::onResetContext(uint32_t resetBits) {
593593
fHWVertexArrayState.invalidate();
594594
fHWBufferState[kVertex_GrBufferType].invalidate();
595595
fHWBufferState[kIndex_GrBufferType].invalidate();
596+
if (this->glCaps().requiresFlushBetweenNonAndInstancedDraws()) {
597+
fRequiresFlushBeforeNextInstancedDraw = true;
598+
}
596599
}
597600

598601
if (resetBits & kRenderTarget_GrGLBackendState) {
@@ -2347,6 +2350,9 @@ void GrGLGpu::flushRenderTargetNoColorWrites(GrGLRenderTarget* target) {
23472350
}
23482351
}
23492352
#endif
2353+
if (this->glCaps().requiresFlushBetweenNonAndInstancedDraws()) {
2354+
fRequiresFlushBeforeNextInstancedDraw = false;
2355+
}
23502356
fHWBoundRenderTargetUniqueID = rtID;
23512357
this->flushViewport(target->getViewport());
23522358
}
@@ -2489,6 +2495,9 @@ void GrGLGpu::sendMeshToGpu(GrPrimitiveType primitiveType, const GrBuffer* verte
24892495
this->setupGeometry(nullptr, vertexBuffer, 0, nullptr, 0, GrPrimitiveRestart::kNo);
24902496
GL_CALL(DrawArrays(glPrimType, baseVertex, vertexCount));
24912497
}
2498+
if (this->glCaps().requiresFlushBetweenNonAndInstancedDraws()) {
2499+
fRequiresFlushBeforeNextInstancedDraw = true;
2500+
}
24922501
fStats.incNumDraws();
24932502
}
24942503

@@ -2508,13 +2517,21 @@ void GrGLGpu::sendIndexedMeshToGpu(GrPrimitiveType primitiveType, const GrBuffer
25082517
} else {
25092518
GL_CALL(DrawElements(glPrimType, indexCount, GR_GL_UNSIGNED_SHORT, indices));
25102519
}
2520+
if (this->glCaps().requiresFlushBetweenNonAndInstancedDraws()) {
2521+
fRequiresFlushBeforeNextInstancedDraw = true;
2522+
}
25112523
fStats.incNumDraws();
25122524
}
25132525

25142526
void GrGLGpu::sendInstancedMeshToGpu(GrPrimitiveType primitiveType, const GrBuffer* vertexBuffer,
25152527
int vertexCount, int baseVertex,
25162528
const GrBuffer* instanceBuffer, int instanceCount,
25172529
int baseInstance) {
2530+
if (fRequiresFlushBeforeNextInstancedDraw) {
2531+
SkASSERT(this->glCaps().requiresFlushBetweenNonAndInstancedDraws());
2532+
GL_CALL(Flush());
2533+
fRequiresFlushBeforeNextInstancedDraw = false;
2534+
}
25182535
GrGLenum glPrimType = gr_primitive_type_to_gl_mode(primitiveType);
25192536
int maxInstances = this->glCaps().maxInstancesPerDrawWithoutCrashing(instanceCount);
25202537
for (int i = 0; i < instanceCount; i += maxInstances) {
@@ -2532,6 +2549,11 @@ void GrGLGpu::sendIndexedInstancedMeshToGpu(GrPrimitiveType primitiveType,
25322549
int baseVertex, const GrBuffer* instanceBuffer,
25332550
int instanceCount, int baseInstance,
25342551
GrPrimitiveRestart enablePrimitiveRestart) {
2552+
if (fRequiresFlushBeforeNextInstancedDraw) {
2553+
SkASSERT(this->glCaps().requiresFlushBetweenNonAndInstancedDraws());
2554+
GL_CALL(Flush());
2555+
fRequiresFlushBeforeNextInstancedDraw = false;
2556+
}
25352557
const GrGLenum glPrimType = gr_primitive_type_to_gl_mode(primitiveType);
25362558
GrGLvoid* indices = reinterpret_cast<void*>(indexBuffer->baseOffset() +
25372559
sizeof(uint16_t) * baseIndex);

src/gpu/gl/GrGLGpu.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,7 @@ class GrGLGpu final : public GrGpu, private GrMesh::SendToGpuImpl {
630630

631631
float fHWMinSampleShading;
632632
GrPrimitiveType fLastPrimitiveType;
633+
bool fRequiresFlushBeforeNextInstancedDraw = false;
633634

634635
class SamplerObjectCache;
635636
std::unique_ptr<SamplerObjectCache> fSamplerObjectCache;

0 commit comments

Comments
 (0)