AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "Reland "Use glDraw.*BaseInstance calls to avoid deferred buffer binding""

Browse Source 
This reverts commit d84b691950.

Reason for revert: seems like chrome's glInterface is maybe not setup correctly. Crashing on intel gpu chrome bots: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8884195221041295328/+/steps/trace_test_on__intel-hd-630-ubuntu-stable__GPU_on_Linux__retry_shards_with_patch__on_linux-intel-stable/0/stdout

snippet: 
Crash reason:  SIGABRT
Crash address: 0x3e8000050b0
Process uptime: not available

Thread 0 (crashed)
 0  libc.so.6 + 0x43ed7
    rax = 0x0000000000000000   rdx = 0x0000000000000000
    rcx = 0x00007ff964c3ced7   rbx = 0x00002cc73685ddc0
    rsi = 0x00007fffb9128960   rdi = 0x0000000000000002
    rbp = 0x00007fffb9128bb0   rsp = 0x00007fffb9128960
     r8 = 0x0000000000000000    r9 = 0x00007fffb9128960
    r10 = 0x0000000000000008   r11 = 0x0000000000000246
    r12 = 0x0000000000000048   r13 = 0x0000000000000000
    r14 = 0x000000000000006c   r15 = 0x0000000000000000
    rip = 0x00007ff964c3ced7
    Found by: given as instruction pointer in context
 1  chrome!GrGLGpu::drawElementsInstancedBaseVertexBaseInstance(GrPrimitiveType, int, unsigned int, void const*, int, int, unsigned int)::$_81::operator()() const + 0x33
    rbp = 0x00007fffb9128bc0   rsp = 0x00007fffb9128bc0
    rip = 0x000055c5c1db6923
    Found by: previous frame's frame pointer
 2  chrome!drawElementsInstancedBaseVertexBaseInstance [GrGLGpu.cpp : 2306 + 0x5]
    rbp = 0x00007fffb9128c10   rsp = 0x00007fffb9128bd0
    rip = 0x000055c5c1db68b6
    Found by: call frame info
 3  chrome!onDrawIndexed [GrGLOpsRenderPass.cpp : 144 + 0x16]

Original change's description:
> Reland "Use glDraw.*BaseInstance calls to avoid deferred buffer binding"
> 
> This is a reland of e8c963d474
> 
> Original change's description:
> > Use glDraw.*BaseInstance calls to avoid deferred buffer binding
> > 
> > Change-Id: I968dab317673051acc65f87ea76a0d657d89b3d2
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279538
> > Commit-Queue: Chris Dalton <csmartdalton@google.com>
> > Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> 
> Change-Id: I79b2d23e5e66d47214898a9068079b6fe2269599
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/280806
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>

TBR=bsalomon@google.com,csmartdalton@google.com,michaelludwig@google.com

Change-Id: I1d2617f077676b6f36bb723615d527e4aa4b3efe
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281057
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Michael Ludwig 2020-04-01 21:55:19 +00:00 committed by Skia Commit-Bot
parent 7c75226854
commit b14b144458
8 changed files with 62 additions and 132 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/gpu/gl/GrGLCaps.cpp
View File

@ -37,7 +37,7 @@ GrGLCaps::GrGLCaps(const GrContextOptions& contextOptions,
fDrawIndirectSupport = false;
fDrawRangeElementsSupport = false;
fMultiDrawIndirectSupport = false;
fBaseVertexBaseInstanceSupport = false;
fBaseInstanceSupport = false;
fUseNonVBOVertexAndIndexDynamicData = false;
fIsCoreProfile = false;
fBindFragDataLocationSupport = false;
@ -625,17 +625,18 @@ void GrGLCaps::init(const GrContextOptions& contextOptions,
if (GR_IS_GR_GL(standard)) {
fDrawIndirectSupport = version >= GR_GL_VER(4,0) ||
ctxInfo.hasExtension("GL_ARB_draw_indirect");
fBaseVertexBaseInstanceSupport = version >= GR_GL_VER(4,2) ||
ctxInfo.hasExtension("GL_ARB_base_instance");
fBaseInstanceSupport = version >= GR_GL_VER(4,2);
fMultiDrawIndirectSupport = version >= GR_GL_VER(4,3) ||
ctxInfo.hasExtension("GL_ARB_multi_draw_indirect");
(fDrawIndirectSupport &&
!fBaseInstanceSupport && // The ARB extension has no base inst.
ctxInfo.hasExtension("GL_ARB_multi_draw_indirect"));
fDrawRangeElementsSupport = version >= GR_GL_VER(2,0);
} else if (GR_IS_GR_GL_ES(standard)) {
fDrawIndirectSupport = version >= GR_GL_VER(3,1);
fMultiDrawIndirectSupport = fDrawIndirectSupport &&
ctxInfo.hasExtension("GL_EXT_multi_draw_indirect");
fBaseVertexBaseInstanceSupport = ctxInfo.hasExtension("GL_EXT_base_instance") ||
ctxInfo.hasExtension("GL_ANGLE_base_vertex_base_instance");
fBaseInstanceSupport = ctxInfo.hasExtension("GL_EXT_base_instance") ||
ctxInfo.hasExtension("GL_ANGLE_base_vertex_base_instance");
fDrawRangeElementsSupport = version >= GR_GL_VER(3,0);
} else if (GR_IS_GR_WEBGL(standard)) {
// WebGL lacks indirect support, but drawRange was added in WebGL 2.0
@ -1183,7 +1184,7 @@ void GrGLCaps::onDumpJSON(SkJSONWriter* writer) const {
writer->appendBool("Debug support", fDebugSupport);
writer->appendBool("Draw indirect support", fDrawIndirectSupport);
writer->appendBool("Multi draw indirect support", fMultiDrawIndirectSupport);
writer->appendBool("Base (vertex base) instance support", fBaseVertexBaseInstanceSupport);
writer->appendBool("Base instance support", fBaseInstanceSupport);
writer->appendBool("RGBA 8888 pixel ops are slow", fRGBA8888PixelsOpsAreSlow);
writer->appendBool("Partial FBO read is slow", fPartialFBOReadIsSlow);
writer->appendBool("Bind uniform location support", fBindUniformLocationSupport);
@ -3543,7 +3544,7 @@ void GrGLCaps::applyDriverCorrectnessWorkarounds(const GrGLContextInfo& ctxInfo,
// http://anglebug.com/4536
if (ctxInfo.driver() == kANGLE_GrGLDriver &&
ctxInfo.angleBackend() != GrGLANGLEBackend::kOpenGL) {
fBaseVertexBaseInstanceSupport = false;
fBaseInstanceSupport = false;
}
// Currently the extension is advertised but fb fetch is broken on 500 series Adrenos like the

7
src/gpu/gl/GrGLCaps.h
View File

@ -324,9 +324,8 @@ public:
/// Is there support for glDrawRangeElements?
bool drawRangeElementsSupport() const { return fDrawRangeElementsSupport; }
/// Are the glDraw*Base(VertexBase)Instance methods, and baseInstance fields in indirect draw
//commands supported?
bool baseVertexBaseInstanceSupport() const { return fBaseVertexBaseInstanceSupport; }
/// Are the baseInstance fields supported in indirect draw commands?
bool baseInstanceSupport() const { return fBaseInstanceSupport; }
/// Use indices or vertices in CPU arrays rather than VBOs for dynamic content.
bool useNonVBOVertexAndIndexDynamicData() const { return fUseNonVBOVertexAndIndexDynamicData; }
@ -529,7 +528,7 @@ private:
bool fDrawIndirectSupport : 1;
bool fDrawRangeElementsSupport : 1;
bool fMultiDrawIndirectSupport : 1;
bool fBaseVertexBaseInstanceSupport : 1;
bool fBaseInstanceSupport : 1;
bool fUseNonVBOVertexAndIndexDynamicData : 1;
bool fIsCoreProfile : 1;
bool fBindFragDataLocationSupport : 1;

24
src/gpu/gl/GrGLGpu.cpp
View File

@ -2274,43 +2274,19 @@ void GrGLGpu::drawRangeElements(GrPrimitiveType primitiveType, GrGLuint minIndex
void GrGLGpu::drawArraysInstanced(GrPrimitiveType primitiveType, GrGLint baseVertex,
GrGLsizei vertexCount, GrGLsizei instanceCount) {
SkASSERT(this->glCaps().instanceAttribSupport());
SkASSERT(instanceCount <= this->glCaps().maxInstancesPerDrawWithoutCrashing(instanceCount));
GrGLenum glPrimType = this->prepareToDraw(primitiveType);
GL_CALL(DrawArraysInstanced(glPrimType, baseVertex, vertexCount, instanceCount));
}
void GrGLGpu::drawArraysInstancedBaseInstance(GrPrimitiveType primitiveType, GrGLint baseVertex,
GrGLsizei vertexCount, GrGLsizei instanceCount,
GrGLuint baseInstance) {
SkASSERT(this->glCaps().instanceAttribSupport());
SkASSERT(this->glCaps().baseVertexBaseInstanceSupport());
SkASSERT(instanceCount <= this->glCaps().maxInstancesPerDrawWithoutCrashing(instanceCount));
GrGLenum glPrimType = this->prepareToDraw(primitiveType);
GL_CALL(DrawArraysInstancedBaseInstance(glPrimType, baseVertex, vertexCount, instanceCount,
baseInstance));
}
void GrGLGpu::drawElementsInstanced(GrPrimitiveType primitiveType, GrGLsizei indexCount,
GrGLenum indexType, const void* indices,
GrGLsizei instanceCount) {
SkASSERT(this->glCaps().instanceAttribSupport());
SkASSERT(instanceCount <= this->glCaps().maxInstancesPerDrawWithoutCrashing(instanceCount));
GrGLenum glPrimType = this->prepareToDraw(primitiveType);
GL_CALL(DrawElementsInstanced(glPrimType, indexCount, indexType, indices, instanceCount));
}
void GrGLGpu::drawElementsInstancedBaseVertexBaseInstance(
GrPrimitiveType primitiveType, GrGLsizei indexCount, GrGLenum indexType,
const void* indices, GrGLsizei instanceCount, GrGLint baseVertex, GrGLuint baseInstance) {
SkASSERT(this->glCaps().instanceAttribSupport());
SkASSERT(this->glCaps().baseVertexBaseInstanceSupport());
SkASSERT(instanceCount <= this->glCaps().maxInstancesPerDrawWithoutCrashing(instanceCount));
GrGLenum glPrimType = this->prepareToDraw(primitiveType);
GL_CALL(DrawElementsInstancedBaseVertexBaseInstance(glPrimType, indexCount, indexType, indices,
instanceCount, baseVertex, baseInstance));
}
void GrGLGpu::onResolveRenderTarget(GrRenderTarget* target, const SkIRect& resolveRect,
ForExternalIO) {
// Some extensions automatically resolves the texture when it is read.

5
src/gpu/gl/GrGLGpu.h
View File

@ -113,13 +113,8 @@ public:
GrGLsizei indexCount, GrGLenum indexType, const void* indices);
void drawArraysInstanced(GrPrimitiveType, GrGLint baseVertex, GrGLsizei vertexCount,
GrGLsizei instanceCount);
void drawArraysInstancedBaseInstance(GrPrimitiveType, GrGLint baseVertex, GrGLsizei vertexCount,
GrGLsizei instanceCount, GrGLuint baseInstance);
void drawElementsInstanced(GrPrimitiveType, GrGLsizei indexCount, GrGLenum indexType,
const void* indices, GrGLsizei instanceCount);
void drawElementsInstancedBaseVertexBaseInstance(
GrPrimitiveType, GrGLsizei indexCount, GrGLenum indexType, const void* indices,
GrGLsizei instanceCount, GrGLint baseVertex, GrGLuint baseInstance);
// The GrGLOpsRenderPass does not buffer up draws before submitting them to the gpu.
// Thus this is the implementation of the clear call for the corresponding passthrough function

5
src/gpu/gl/GrGLInterfaceAutogen.cpp
View File

@ -240,10 +240,7 @@ bool GrGLInterface::validate() const {
(GR_IS_GR_GL_ES(fStandard) && (
fExtensions.has("GL_EXT_base_instance") ||
fExtensions.has("GL_ANGLE_base_vertex_base_instance")))) {
if (!fFunctions.fDrawArraysInstancedBaseInstance ||
!fFunctions.fDrawElementsInstancedBaseVertexBaseInstance) {
RETURN_FALSE_INTERFACE;
}
// all functions were marked optional or test_only
}
if (GR_IS_GR_GL(fStandard) ||

129
src/gpu/gl/GrGLOpsRenderPass.cpp
View File

@ -48,7 +48,9 @@ bool GrGLOpsRenderPass::onBindTextures(const GrPrimitiveProcessor& primProc,
const GrSurfaceProxy* const primProcTextures[],
const GrPipeline& pipeline) {
GrGLProgram* program = fGpu->currentProgram();
SkASSERT(program);
if (!program) {
return false;
}
program->bindTextures(primProc, primProcTextures, pipeline);
return true;
}
@ -58,7 +60,9 @@ void GrGLOpsRenderPass::onBindBuffers(const GrBuffer* indexBuffer, const GrBuffe
GrPrimitiveRestart primitiveRestart) {
SkASSERT((primitiveRestart == GrPrimitiveRestart::kNo) || indexBuffer);
GrGLProgram* program = fGpu->currentProgram();
SkASSERT(program);
if (!program) {
return;
}
int numAttribs = program->numVertexAttributes() + program->numInstanceAttributes();
fAttribArrayState = fGpu->bindInternalVertexArray(indexBuffer, numAttribs, primitiveRestart);
@ -72,27 +76,32 @@ void GrGLOpsRenderPass::onBindBuffers(const GrBuffer* indexBuffer, const GrBuffe
}
}
if (!fGpu->glCaps().baseVertexBaseInstanceSupport()) {
// This platform does not support baseInstance. Defer binding of the instance buffer.
fActiveInstanceBuffer = sk_ref_sp(instanceBuffer);
} else {
this->bindInstanceBuffer(instanceBuffer, 0);
}
if (!indexBuffer && fGpu->glCaps().drawArraysBaseVertexIsBroken()) {
// There is a driver bug affecting glDrawArrays. Defer binding of the vertex buffer.
fActiveVertexBuffer = sk_ref_sp(vertexBuffer);
} else if (indexBuffer && !fGpu->glCaps().baseVertexBaseInstanceSupport()) {
// This platform does not support baseVertex with indexed draws. Defer binding of the
// vertex buffer.
fActiveVertexBuffer = sk_ref_sp(vertexBuffer);
} else {
this->bindVertexBuffer(vertexBuffer, 0);
}
// We defer binding of instance and vertex buffers because GL does not (always) support base
// instance and/or base vertex.
fActiveInstanceBuffer = sk_ref_sp(instanceBuffer);
fActiveVertexBuffer = sk_ref_sp(vertexBuffer);
}
void GrGLOpsRenderPass::bindInstanceBuffer(const GrBuffer* instanceBuffer, int baseInstance) {
void GrGLOpsRenderPass::setupGeometry(const GrBuffer* vertexBuffer, int baseVertex,
const GrBuffer* instanceBuffer, int baseInstance) {
GrGLProgram* program = fGpu->currentProgram();
SkASSERT(program);
if (!program) {
return;
}
if (int vertexStride = program->vertexStride()) {
SkASSERT(vertexBuffer);
SkASSERT(vertexBuffer->isCpuBuffer() ||
!static_cast<const GrGpuBuffer*>(vertexBuffer)->isMapped());
size_t bufferOffset = baseVertex * static_cast<size_t>(vertexStride);
for (int i = 0; i < program->numVertexAttributes(); ++i) {
const auto& attrib = program->vertexAttribute(i);
static constexpr int kDivisor = 0;
fAttribArrayState->set(fGpu, attrib.fLocation, vertexBuffer, attrib.fCPUType,
attrib.fGPUType, vertexStride, bufferOffset + attrib.fOffset,
kDivisor);
}
}
if (int instanceStride = program->instanceStride()) {
SkASSERT(instanceBuffer);
SkASSERT(instanceBuffer->isCpuBuffer() ||
@ -109,47 +118,20 @@ void GrGLOpsRenderPass::bindInstanceBuffer(const GrBuffer* instanceBuffer, int b
}
}
void GrGLOpsRenderPass::bindVertexBuffer(const GrBuffer* vertexBuffer, int baseVertex) {
GrGLProgram* program = fGpu->currentProgram();
SkASSERT(program);
if (int vertexStride = program->vertexStride()) {
SkASSERT(vertexBuffer);
SkASSERT(vertexBuffer->isCpuBuffer() ||
!static_cast<const GrGpuBuffer*>(vertexBuffer)->isMapped());
size_t bufferOffset = baseVertex * static_cast<size_t>(vertexStride);
for (int i = 0; i < program->numVertexAttributes(); ++i) {
const auto& attrib = program->vertexAttribute(i);
static constexpr int kDivisor = 0;
fAttribArrayState->set(fGpu, attrib.fLocation, vertexBuffer, attrib.fCPUType,
attrib.fGPUType, vertexStride, bufferOffset + attrib.fOffset,
kDivisor);
}
}
}
void GrGLOpsRenderPass::onDraw(int vertexCount, int baseVertex) {
SkASSERT(!fActiveVertexBuffer || fGpu->glCaps().drawArraysBaseVertexIsBroken());
if (fGpu->glCaps().drawArraysBaseVertexIsBroken()) {
this->bindVertexBuffer(fActiveVertexBuffer.get(), baseVertex);
baseVertex = 0;
this->setupGeometry(fActiveVertexBuffer.get(), baseVertex, nullptr, 0);
fGpu->drawArrays(fPrimitiveType, 0, vertexCount);
return;
}
this->setupGeometry(fActiveVertexBuffer.get(), 0, nullptr, 0);
fGpu->drawArrays(fPrimitiveType, baseVertex, vertexCount);
}
void GrGLOpsRenderPass::onDrawIndexed(int indexCount, int baseIndex, uint16_t minIndexValue,
uint16_t maxIndexValue, int baseVertex) {
if (fGpu->glCaps().baseVertexBaseInstanceSupport()) {
SkASSERT(!fActiveVertexBuffer);
if (baseVertex != 0) {
fGpu->drawElementsInstancedBaseVertexBaseInstance(
fPrimitiveType, indexCount, GR_GL_UNSIGNED_SHORT,
this->offsetForBaseIndex(baseIndex), 1, baseVertex, 0);
return;
}
} else {
this->bindVertexBuffer(fActiveVertexBuffer.get(), baseVertex);
}
this->setupGeometry(fActiveVertexBuffer.get(), baseVertex, nullptr, 0);
if (fGpu->glCaps().drawRangeElementsSupport()) {
fGpu->drawRangeElements(fPrimitiveType, minIndexValue, maxIndexValue, indexCount,
GR_GL_UNSIGNED_SHORT, this->offsetForBaseIndex(baseIndex));
@ -161,25 +143,12 @@ void GrGLOpsRenderPass::onDrawIndexed(int indexCount, int baseIndex, uint16_t mi
void GrGLOpsRenderPass::onDrawInstanced(int instanceCount, int baseInstance, int vertexCount,
int baseVertex) {
SkASSERT(!fActiveVertexBuffer || fGpu->glCaps().drawArraysBaseVertexIsBroken());
if (fGpu->glCaps().drawArraysBaseVertexIsBroken()) {
// We weren't able to bind the vertex buffer during onBindBuffers because of a driver bug
// affecting glDrawArrays.
this->bindVertexBuffer(fActiveVertexBuffer.get(), 0);
}
int maxInstances = fGpu->glCaps().maxInstancesPerDrawWithoutCrashing(instanceCount);
for (int i = 0; i < instanceCount; i += maxInstances) {
int instanceCountForDraw = std::min(instanceCount - i, maxInstances);
int baseInstanceForDraw = baseInstance + i;
if (fGpu->glCaps().baseVertexBaseInstanceSupport()) {
SkASSERT(!fActiveInstanceBuffer);
fGpu->drawArraysInstancedBaseInstance(fPrimitiveType, baseVertex, vertexCount,
instanceCountForDraw, baseInstanceForDraw);
} else {
this->bindInstanceBuffer(fActiveInstanceBuffer.get(), baseInstanceForDraw);
fGpu->drawArraysInstanced(fPrimitiveType, baseVertex, vertexCount,
instanceCountForDraw);
}
this->setupGeometry(fActiveVertexBuffer.get(), 0, fActiveInstanceBuffer.get(),
baseInstance + i);
fGpu->drawArraysInstanced(fPrimitiveType, baseVertex, vertexCount,
std::min(instanceCount - i, maxInstances));
}
}
@ -187,21 +156,11 @@ void GrGLOpsRenderPass::onDrawIndexedInstanced(int indexCount, int baseIndex, in
int baseInstance, int baseVertex) {
int maxInstances = fGpu->glCaps().maxInstancesPerDrawWithoutCrashing(instanceCount);
for (int i = 0; i < instanceCount; i += maxInstances) {
int instanceCountForDraw = std::min(instanceCount - i, maxInstances);
int baseInstanceForDraw = baseInstance + i;
if (fGpu->glCaps().baseVertexBaseInstanceSupport()) {
SkASSERT(!fActiveInstanceBuffer);
SkASSERT(!fActiveVertexBuffer);
fGpu->drawElementsInstancedBaseVertexBaseInstance(
fPrimitiveType, indexCount, GR_GL_UNSIGNED_SHORT,
this->offsetForBaseIndex(baseIndex), instanceCountForDraw, baseVertex,
baseInstanceForDraw);
} else {
this->bindInstanceBuffer(fActiveInstanceBuffer.get(), baseInstanceForDraw);
this->bindVertexBuffer(fActiveVertexBuffer.get(), baseVertex);
fGpu->drawElementsInstanced(fPrimitiveType, indexCount, GR_GL_UNSIGNED_SHORT,
this->offsetForBaseIndex(baseIndex), instanceCountForDraw);
}
this->setupGeometry(fActiveVertexBuffer.get(), baseVertex, fActiveInstanceBuffer.get(),
baseInstance + i);
fGpu->drawElementsInstanced(fPrimitiveType, indexCount, GR_GL_UNSIGNED_SHORT,
this->offsetForBaseIndex(baseIndex),
std::min(instanceCount - i, maxInstances));
}
}

4
src/gpu/gl/GrGLOpsRenderPass.h
View File

@ -40,8 +40,8 @@ public:
private:
GrGpu* gpu() override { return fGpu; }
void bindInstanceBuffer(const GrBuffer*, int baseInstance);
void bindVertexBuffer(const GrBuffer*, int baseVertex);
void setupGeometry(const GrBuffer* vertexBuffer, int baseVertex, const GrBuffer* instanceBuffer,
int baseInstance);
const void* offsetForBaseIndex(int baseIndex) const {
if (!fIndexPointer) {

3
tools/gpu/gl/interface/interface.json5
View File

@ -179,6 +179,9 @@
"functions": [
"DrawArraysInstancedBaseInstance", "DrawElementsInstancedBaseVertexBaseInstance"
],
"optional": [
"DrawArraysInstancedBaseInstance", "DrawElementsInstancedBaseVertexBaseInstance"
],
},
{ // ES 3.0 has glDrawBuffers but not glDrawBuffer