Browse Source

GL: fix Framebuffer::implementationColorRead*() to follow spec.

Not sure what I thought here, but using GL_IMPLEMENTATION_COLOR_READ_*
with glGetFramebufferParameteriv() is possible only since GL 4.5
(there's no extension adding it) and there's no equivalent for it in
GLES 3.1. I assumed that since the entry point is defined in 3.1 then it
would also accept the same enum, but apparently not -- and this
inconsistency is not fixed in 3.2 either.

Funnily enough, all drivers I tried (Mesa ES, SwiftShader, ANGLE) were
accepting this. The first driver that complained was ARM Mali on Huawei
P10 and there I assumed a driver bug -- but on the contrary, this is the
only driver following the spec properly :)

Now the glGetFramebufferParameteriv() API (and the DSA equivalent) is
used only on GL 4.5 contexts and up, everywhere else it's done using
the old-style glGetInteger(). I also renamed the implementations to not
misleadingly mention ES 3.1, since it has nothing to do with this. The
code path dispatch code is a bit entangled due to all the workarounds,
hopefully not breaking any behavior that worked before.
pull/331/head
Vladimír Vondruš 7 years ago
parent
commit
a1ed7f4863
  1. 14
      src/Magnum/GL/AbstractFramebuffer.cpp
  2. 10
      src/Magnum/GL/AbstractFramebuffer.h
  3. 68
      src/Magnum/GL/Implementation/FramebufferState.cpp

14
src/Magnum/GL/AbstractFramebuffer.cpp

@ -221,35 +221,33 @@ PixelType AbstractFramebuffer::implementationColorReadType() {
return PixelType((this->*Context::current().state().framebuffer->implementationColorReadFormatTypeImplementation)(GL_IMPLEMENTATION_COLOR_READ_TYPE));
}
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationDefault(const GLenum what) {
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationGlobal(const GLenum what) {
bindInternal(FramebufferTarget::Read);
GLint formatType;
glGetIntegerv(what, &formatType);
return formatType;
}
#if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL)
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationES31(const GLenum what) {
#ifndef MAGNUM_TARGET_GLES
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationFramebuffer(const GLenum what) {
const FramebufferTarget target = bindInternal();
GLint formatType;
glGetFramebufferParameteriv(GLenum(target), what, &formatType);
return formatType;
}
#endif
#ifndef MAGNUM_TARGET_GLES
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationES31DSA(const GLenum what) {
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationFramebufferDSA(const GLenum what) {
GLint formatType;
glGetNamedFramebufferParameteriv(_id, what, &formatType);
return formatType;
}
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationES31DSAMesa(const GLenum what) {
GLenum AbstractFramebuffer::implementationColorReadFormatTypeImplementationFramebufferDSAMesa(const GLenum what) {
/* Mesa needs the framebuffer bound for read even with DSA. See the
"mesa-implementation-color-read-format-dsa-explicit-binding" workaround
for details. */
bindInternal(FramebufferTarget::Read);
return implementationColorReadFormatTypeImplementationES31DSA(what);
return implementationColorReadFormatTypeImplementationFramebufferDSA(what);
}
#endif

10
src/Magnum/GL/AbstractFramebuffer.h

@ -772,13 +772,11 @@ class MAGNUM_GL_EXPORT AbstractFramebuffer {
FramebufferTarget MAGNUM_GL_LOCAL bindImplementationSingle();
#endif
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationDefault(GLenum what);
#if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL)
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationES31(GLenum what);
#endif
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationGlobal(GLenum what);
#ifndef MAGNUM_TARGET_GLES
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationES31DSA(GLenum what);
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationES31DSAMesa(GLenum what);
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationFramebuffer(GLenum what);
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationFramebufferDSA(GLenum what);
GLenum MAGNUM_GL_LOCAL implementationColorReadFormatTypeImplementationFramebufferDSAMesa(GLenum what);
#endif
GLenum MAGNUM_GL_LOCAL checkStatusImplementationDefault(FramebufferTarget target);

68
src/Magnum/GL/Implementation/FramebufferState.cpp

@ -256,50 +256,46 @@ FramebufferState::FramebufferState(Context& context, std::vector<std::string>& e
/* Implementation-specific color read format/type implementation */
#ifndef MAGNUM_TARGET_GLES
if(context.isExtensionSupported<Extensions::ARB::direct_state_access>() && context.isExtensionSupported<Extensions::ARB::ES3_1_compatibility>()) {
/* Get(Named)FramebufferParameteriv() supports querying
GL_IMPLEMENTATION_COLOR_READ_{FORMAT,TYPE} since GL 4.5. No
corresponding extension enabling this, only a mention of Bug 12360
that's supposed to have more information about this. But the Khronos
bugzilla is lost to internet history now and everything gets redirected
to the mostly-empty GitHub issue tracker (and it doesn't even have the
old bugs imported), so this is all I got. The whole thing is a
clusterfuck:
- ES3.1 adds GetFramebufferParameteriv() but it *doesn't* allow
GL_IMPLEMENTATION_COLOR_READ_FORMAT to be used with it. ES3.2
doesn't fix that omission either. Funnily enough, most drivers
(including NV, Mesa and SwiftShader) support such a query, the only
driver which doesn't (and thus matches the spec) is on my Huawei
P10. What.
- Intel implementation on Windows, even though supporting 4.5 and
DSA, returns absolute garbage on everything except the most basic
GetInteger query
- NVidia returns broken values when calling the DSA code path
- Mesa needs the framebuffer to be bound even for DSA queries */
if(context.isVersionSupported(Version::GL450)
#ifdef CORRADE_TARGET_WINDOWS
if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) &&
!context.isDriverWorkaroundDisabled("intel-windows-implementation-color-read-format-completely-broken")) {
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationDefault;
} else
&& !((context.detectedDriver() & Context::DetectedDriver::IntelWindows) &&
!context.isDriverWorkaroundDisabled("intel-windows-implementation-color-read-format-completely-broken"))
#endif
{
) {
if(context.isExtensionSupported<Extensions::ARB::direct_state_access>()
&& !((context.detectedDriver() & Context::DetectedDriver::NVidia) && !context.isDriverWorkaroundDisabled("nv-implementation-color-read-format-dsa-broken"))
) {
/* DSA extension added above */
extensions.push_back(Extensions::ARB::ES3_1_compatibility::string());
if((context.detectedDriver() & Context::DetectedDriver::NVidia) &&
!context.isDriverWorkaroundDisabled("nv-implementation-color-read-format-dsa-broken")) {
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationES31;
} else if((context.detectedDriver() & Context::DetectedDriver::Mesa) &&
!context.isDriverWorkaroundDisabled("mesa-implementation-color-read-format-dsa-explicit-binding")) {
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationES31DSAMesa;
} else {
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationES31DSA;
}
}
} else
#endif
#if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL)
/* Checks for 3.1 on ES and for ARB_ES3_1_compatibility on desktop */
if(context.isVersionSupported(Version::GLES310)) {
#ifdef CORRADE_TARGET_WINDOWS
if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) &&
!context.isDriverWorkaroundDisabled("intel-windows-implementation-color-read-format-completely-broken")) {
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationDefault;
} else
#endif
{
#ifndef MAGNUM_TARGET_GLES
extensions.push_back(Extensions::ARB::ES3_1_compatibility::string());
#endif
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationES31;
if((context.detectedDriver() & Context::DetectedDriver::Mesa) && !context.isDriverWorkaroundDisabled("mesa-implementation-color-read-format-dsa-explicit-binding"))
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationFramebufferDSAMesa;
else implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationFramebufferDSA;
} else {
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationFramebuffer;
}
} else
#endif
{
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationDefault;
implementationColorReadFormatTypeImplementation = &AbstractFramebuffer::implementationColorReadFormatTypeImplementationGlobal;
}
/* Framebuffer reading implementation in desktop/ES */

Loading…
Cancel
Save