Skip to content

Commit

Permalink
GL: pile on more workarounds for CubeMapTexture DSA code paths.
Browse files Browse the repository at this point in the history
Ugh why this has to be SO TERRIBLE. I'm starting to have extremely low
expectations for similar parts of Vulkan drivers by the same vendors.
  • Loading branch information
mosra committed Oct 19, 2019
1 parent f2ba560 commit ca326fd
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 85 deletions.
3 changes: 3 additions & 0 deletions doc/changelog.dox
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ See also:
- @cpp "intel-windows-broken-dsa-integer-vertex-attributes" @ce fixing
@ref GL::Mesh::addVertexBuffer() with @ref Magnum::Short "Short"
attributes
- New @cpp "amd-windows-cubemap-image3d-slice-by-slice" @ce workaround for
broken handling of cube map image download and upload in DSA APIs on AMD
Windows drivers
- New @cpp "arm-mali-timer-queries-oom-in-shell" @ce workaround for
@ref GL::Context::DetectedDriver::ArmMali "ARM Mali" drivers on Android
disabling the @gl_extension{EXT,disjoint_timer_query} extension in Android
Expand Down
40 changes: 31 additions & 9 deletions src/Magnum/GL/CubeMapTexture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void CubeMapTexture::image(const Int level, Image3D& image) {

Buffer::unbindInternal(Buffer::TargetHint::PixelPack);
Context::current().state().renderer->applyPixelStoragePack(image.storage());
glGetTextureImage(_id, level, GLenum(pixelFormat(image.format())), GLenum(pixelType(image.format(), image.formatExtra())), data.size(), data);
(this->*Context::current().state().texture->getFullCubeImageImplementation)(level, size, pixelFormat(image.format()), pixelType(image.format(), image.formatExtra()), data.size(), data, image.storage());
image = Image3D{image.storage(), image.format(), image.formatExtra(), image.pixelSize(), size, std::move(data)};
}

Expand All @@ -86,17 +86,15 @@ Image3D CubeMapTexture::image(const Int level, Image3D&& image) {
}

void CubeMapTexture::image(const Int level, const MutableImageView3D& image) {
#ifndef CORRADE_NO_ASSERT
const Vector3i size{imageSize(level), 6};
CORRADE_ASSERT(image.data().data() != nullptr || !size.product(),
"GL::CubeMapTexture::image(): image view is nullptr", );
CORRADE_ASSERT(image.size() == size,
"GL::CubeMapTexture::image(): expected image view size" << size << "but got" << image.size(), );
#endif

Buffer::unbindInternal(Buffer::TargetHint::PixelPack);
Context::current().state().renderer->applyPixelStoragePack(image.storage());
glGetTextureImage(_id, level, GLenum(pixelFormat(image.format())), GLenum(pixelType(image.format(), image.formatExtra())), image.data().size(), image.data());
(this->*Context::current().state().texture->getFullCubeImageImplementation)(level, size, pixelFormat(image.format()), pixelType(image.format(), image.formatExtra()), image.data().size(), image.data(), image.storage());
}

void CubeMapTexture::image(const Int level, BufferImage3D& image, const BufferUsage usage) {
Expand All @@ -113,7 +111,7 @@ void CubeMapTexture::image(const Int level, BufferImage3D& image, const BufferUs

image.buffer().bindInternal(Buffer::TargetHint::PixelPack);
Context::current().state().renderer->applyPixelStoragePack(image.storage());
glGetTextureImage(_id, level, GLenum(image.format()), GLenum(image.type()), dataSize, nullptr);
(this->*Context::current().state().texture->getFullCubeImageImplementation)(level, size, image.format(), image.type(), dataSize, nullptr, image.storage());
}

BufferImage3D CubeMapTexture::image(const Int level, BufferImage3D&& image, const BufferUsage usage) {
Expand Down Expand Up @@ -500,7 +498,7 @@ CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& off

image.buffer().bindInternal(Buffer::TargetHint::PixelUnpack);
Context::current().state().renderer->applyPixelStorageUnpack(image.storage());
glTextureSubImage3D(_id, level, offset.x(), offset.y(), offset.z(), image.size().x(), image.size().y(), image.size().z(), GLenum(image.format()), GLenum(image.type()), nullptr);
(this->*Context::current().state().texture->cubeSubImage3DImplementation)(level, offset, image.size(), image.format(), image.type(), nullptr, image.storage());
return *this;
}

Expand Down Expand Up @@ -613,6 +611,24 @@ GLint CubeMapTexture::getLevelCompressedImageSizeImplementationDSANonImmutableWo
#endif

#ifndef MAGNUM_TARGET_GLES
void CubeMapTexture::getImageImplementationDSA(const GLint level, const Vector3i&, const PixelFormat format, const PixelType type, const std::size_t dataSize, GLvoid* const data, const PixelStorage&) {
glGetTextureImage(_id, level, GLenum(format), GLenum(type), dataSize, data);
}

void CubeMapTexture::getImageImplementationDSAAmdSliceBySlice(const GLint level, const Vector3i& size, const PixelFormat format, const PixelType type, std::size_t, GLvoid* const data, const PixelStorage& storage) {
auto dataProperties = storage.dataProperties(pixelSize(format, type), size);
const std::size_t stride = dataProperties.second.xy().product();
for(Int i = 0; i != size.z(); ++i)
glGetTextureSubImage(_id, level, 0, 0, i, size.x(), size.y(), 1, GLenum(format), GLenum(type), stride, static_cast<char*>(data) + dataProperties.first.sum() + stride*i);
}

void CubeMapTexture::getImageImplementationSliceBySlice(const GLint level, const Vector3i& size, const PixelFormat format, const PixelType type, std::size_t, GLvoid* const data, const PixelStorage& storage) {
auto dataProperties = storage.dataProperties(pixelSize(format, type), size);
const std::size_t stride = dataProperties.second.xy().product();
for(Int i = 0; i != size.z(); ++i)
getImageImplementationDefault(CubeMapCoordinate(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i), level, size.xy(), format, type, stride, static_cast<char*>(data) + stride*i);
}

void CubeMapTexture::getCompressedImageImplementationDSA(const GLint level, const Vector2i&, const std::size_t dataOffset, const std::size_t dataSize, GLvoid* const data) {
glGetCompressedTextureImage(_id, level, dataOffset + dataSize, data);
}
Expand Down Expand Up @@ -654,14 +670,20 @@ void CubeMapTexture::getCompressedImageImplementationRobustness(const CubeMapCoo
#endif

#ifndef MAGNUM_TARGET_GLES
void CubeMapTexture::subImageImplementationDefault(const GLint level, const Vector3i& offset, const Vector3i& size, const PixelFormat format, const PixelType type, const GLvoid* const data, const PixelStorage&) {
void CubeMapTexture::subImageImplementationDSA(const GLint level, const Vector3i& offset, const Vector3i& size, const PixelFormat format, const PixelType type, const GLvoid* const data, const PixelStorage&) {
glTextureSubImage3D(_id, level, offset.x(), offset.y(), offset.z(), size.x(), size.y(), size.z(), GLenum(format), GLenum(type), data);
}

void CubeMapTexture::subImageImplementationSvga3DSliceBySlice(const GLint level, const Vector3i& offset, const Vector3i& size, const PixelFormat format, const PixelType type, const GLvoid* const data, const PixelStorage& storage) {
void CubeMapTexture::subImageImplementationDSASliceBySlice(const GLint level, const Vector3i& offset, const Vector3i& size, const PixelFormat format, const PixelType type, const GLvoid* const data, const PixelStorage& storage) {
const std::size_t stride = std::get<1>(storage.dataProperties(pixelSize(format, type), size)).xy().product();
for(Int i = 0; i != size.z(); ++i)
subImageImplementationDSA(level, {offset.xy(), offset.z() + i}, {size.xy(), 1}, format, type, static_cast<const char*>(data) + stride*i, storage);
}

void CubeMapTexture::subImageImplementationSliceBySlice(const GLint level, const Vector3i& offset, const Vector3i& size, const PixelFormat format, const PixelType type, const GLvoid* const data, const PixelStorage& storage) {
const std::size_t stride = std::get<1>(storage.dataProperties(pixelSize(format, type), size)).xy().product();
for(Int i = 0; i != size.z(); ++i)
subImageImplementationDefault(level, {offset.xy(), offset.z() + i}, {size.xy(), 1}, format, type, static_cast<const char*>(data) + stride*i, storage);
subImageImplementationDefault(CubeMapCoordinate(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i), level, offset.xy(), size.xy(), format, type, static_cast<const char*>(data) + stride*i);
}
#endif

Expand Down
11 changes: 8 additions & 3 deletions src/Magnum/GL/CubeMapTexture.h
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,10 @@ class MAGNUM_GL_EXPORT CubeMapTexture: public AbstractTexture {
#endif

#ifndef MAGNUM_TARGET_GLES
void MAGNUM_GL_LOCAL getImageImplementationDSA(GLint level, const Vector3i& size, PixelFormat format, PixelType type, std::size_t dataSize, GLvoid* data, const PixelStorage& storage);
void MAGNUM_GL_LOCAL getImageImplementationDSAAmdSliceBySlice(GLint level, const Vector3i& size, PixelFormat format, PixelType type, std::size_t dataSize, GLvoid* data, const PixelStorage& storage);
void MAGNUM_GL_LOCAL getImageImplementationSliceBySlice(GLint level, const Vector3i& size, PixelFormat format, PixelType type, std::size_t dataSize, GLvoid* data, const PixelStorage& storage);

void MAGNUM_GL_LOCAL getCompressedImageImplementationDSA(GLint level, const Vector2i& size, std::size_t dataOffset, std::size_t dataSize, GLvoid* data);
void MAGNUM_GL_LOCAL getCompressedImageImplementationDSASingleSliceWorkaround(GLint level, const Vector2i& size, std::size_t dataOffset, std::size_t dataSize, GLvoid* data);

Expand All @@ -1224,9 +1228,10 @@ class MAGNUM_GL_EXPORT CubeMapTexture: public AbstractTexture {
void MAGNUM_GL_LOCAL getCompressedImageImplementationRobustness(CubeMapCoordinate coordinate, GLint level, const Vector2i& size, std::size_t dataSize, GLvoid* data);
#endif

void MAGNUM_GL_LOCAL subImageImplementationDefault(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&);
#ifndef MAGNUM_TARGET_WEBGL
void MAGNUM_GL_LOCAL subImageImplementationSvga3DSliceBySlice(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&);
#ifndef MAGNUM_TARGET_GLES
void MAGNUM_GL_LOCAL subImageImplementationDSA(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&);
void MAGNUM_GL_LOCAL subImageImplementationDSASliceBySlice(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&);
void MAGNUM_GL_LOCAL subImageImplementationSliceBySlice(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&);
#endif

void MAGNUM_GL_LOCAL subImageImplementationDefault(CubeMapCoordinate coordinate, GLint level, const Vector2i& offset, const Vector2i& size, PixelFormat format, PixelType type, const GLvoid* data);
Expand Down
76 changes: 58 additions & 18 deletions src/Magnum/GL/Implementation/TextureState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,20 +176,30 @@ TextureState::TextureState(Context& context, std::vector<std::string>& extension
#endif
}

/* DSA/non-DSA implementation for cubemaps, because Intel Windows drivers
have to be broken in a special way */
/* DSA/non-DSA implementation for cubemaps, because Intel (and AMD) Windows
drivers have to be broken in a special way */
#ifndef MAGNUM_TARGET_GLES
if(context.isExtensionSupported<Extensions::ARB::direct_state_access>()
if(context.isExtensionSupported<Extensions::ARB::direct_state_access>()) {

#ifdef CORRADE_TARGET_WINDOWS
&& (!(context.detectedDriver() & Context::DetectedDriver::IntelWindows) ||
context.isDriverWorkaroundDisabled("intel-windows-broken-dsa-for-cubemaps"))
if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) && !context.isDriverWorkaroundDisabled("intel-windows-broken-dsa-for-cubemaps")) {
getCubeLevelParameterivImplementation = &CubeMapTexture::getLevelParameterImplementationDefault;
cubeSubImageImplementation = &CubeMapTexture::subImageImplementationDefault;
cubeCompressedSubImageImplementation = &CubeMapTexture::compressedSubImageImplementationDefault;
} else if((context.detectedDriver() & Context::DetectedDriver::Amd) && !context.isDriverWorkaroundDisabled("amd-windows-cubemap-image3d-slice-by-slice")) {
/* This one is not broken, but the others are */
getCubeLevelParameterivImplementation = &CubeMapTexture::getLevelParameterImplementationDSA;
cubeSubImageImplementation = &CubeMapTexture::subImageImplementationDefault;
cubeCompressedSubImageImplementation = &CubeMapTexture::compressedSubImageImplementationDefault;
} else
#endif
) {
/* Extension name added above */
{
/* Extension name added above */

getCubeLevelParameterivImplementation = &CubeMapTexture::getLevelParameterImplementationDSA;
cubeSubImageImplementation = &CubeMapTexture::subImageImplementationDSA;
cubeCompressedSubImageImplementation = &CubeMapTexture::compressedSubImageImplementationDSA;
getCubeLevelParameterivImplementation = &CubeMapTexture::getLevelParameterImplementationDSA;
cubeSubImageImplementation = &CubeMapTexture::subImageImplementationDSA;
cubeCompressedSubImageImplementation = &CubeMapTexture::compressedSubImageImplementationDSA;
}
} else
#endif
{
Expand Down Expand Up @@ -299,6 +309,17 @@ TextureState::TextureState(Context& context, std::vector<std::string>& extension
getFullCompressedCubeImageImplementation = &CubeMapTexture::getCompressedImageImplementationDSASingleSliceWorkaround;
else
getFullCompressedCubeImageImplementation = &CubeMapTexture::getCompressedImageImplementationDSA;

if((context.detectedDriver() & Context::DetectedDriver::Amd) &&
context.isExtensionSupported<Extensions::ARB::direct_state_access>() &&
!context.isDriverWorkaroundDisabled("amd-windows-cubemap-image3d-slice-by-slice"))
getFullCubeImageImplementation = &CubeMapTexture::getImageImplementationDSAAmdSliceBySlice;
else if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) &&
context.isExtensionSupported<Extensions::ARB::direct_state_access>() &&
!context.isDriverWorkaroundDisabled("intel-windows-broken-dsa-for-cubemaps"))
getFullCubeImageImplementation = &CubeMapTexture::getImageImplementationSliceBySlice;
else
getFullCubeImageImplementation = &CubeMapTexture::getImageImplementationDSA;
#endif

/* Texture storage implementation for desktop and ES */
Expand Down Expand Up @@ -417,17 +438,14 @@ TextureState::TextureState(Context& context, std::vector<std::string>& extension
subImage2DImplementation = &AbstractTexture::subImageImplementationSvga3DSliceBySlice<&AbstractTexture::subImage2DImplementationDSA>;
#endif
subImage3DImplementation = &AbstractTexture::subImageImplementationSvga3DSliceBySlice<&AbstractTexture::subImage3DImplementationDSA>;
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSvga3DSliceBySlice;

} else
#endif
{
#ifndef MAGNUM_TARGET_GLES
subImage2DImplementation = &AbstractTexture::subImageImplementationSvga3DSliceBySlice<&AbstractTexture::subImage2DImplementationDefault>;
#endif
subImage3DImplementation = &AbstractTexture::subImageImplementationSvga3DSliceBySlice<&AbstractTexture::subImage3DImplementationDefault>;
#ifndef MAGNUM_TARGET_GLES
cubeSubImage3DImplementation = nullptr;
#endif
}
} else
#endif
Expand All @@ -437,12 +455,34 @@ TextureState::TextureState(Context& context, std::vector<std::string>& extension
#if !(defined(MAGNUM_TARGET_WEBGL) && defined(MAGNUM_TARGET_GLES2))
image3DImplementation = &AbstractTexture::imageImplementationDefault;
#endif
/* The other subImage implementations were set already above */
#ifndef MAGNUM_TARGET_GLES
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationDefault;
#endif
}

#ifndef MAGNUM_TARGET_GLES
/* SVGA3D and Intel workaround for cube map texture upload. Overrides the
DSA / non-DSA function pointers set above. */
if((context.detectedDriver() & Context::DetectedDriver::Svga3D) &&
!context.isDriverWorkaroundDisabled("svga3d-texture-upload-slice-by-slice")) {
if(context.isExtensionSupported<Extensions::ARB::direct_state_access>()) {
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationDSASliceBySlice;
} else {
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSliceBySlice;
}
} else if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) &&
!context.isDriverWorkaroundDisabled("intel-windows-broken-dsa-for-cubemaps")) {
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSliceBySlice;
} else if((context.detectedDriver() & Context::DetectedDriver::Amd) &&
!context.isDriverWorkaroundDisabled("amd-windows-cubemap-image3d-slice-by-slice")) {
/* DSA version is broken (non-zero Z offset not allowed), need to
emulate using classic APIs */
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSliceBySlice;
} else if(context.isExtensionSupported<Extensions::ARB::direct_state_access>()) {
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationDSA;
} else
{
cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSliceBySlice;
}
#endif

/* Allocate texture bindings array to hold all possible texture units */
glGetIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &maxTextureUnits);
CORRADE_INTERNAL_ASSERT(maxTextureUnits > 0);
Expand Down
1 change: 1 addition & 0 deletions src/Magnum/GL/Implementation/TextureState.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ struct TextureState {
#ifndef MAGNUM_TARGET_GLES
GLint(CubeMapTexture::*getCubeLevelCompressedImageSizeImplementation)(GLint);
void(CubeMapTexture::*getCubeImageImplementation)(CubeMapCoordinate, GLint, const Vector2i&, PixelFormat, PixelType, std::size_t, GLvoid*);
void(CubeMapTexture::*getFullCubeImageImplementation)(GLint, const Vector3i&, PixelFormat, PixelType, std::size_t, GLvoid*, const PixelStorage&);
void(CubeMapTexture::*getFullCompressedCubeImageImplementation)(GLint, const Vector2i&, std::size_t, std::size_t, GLvoid*);
void(CubeMapTexture::*getCompressedCubeImageImplementation)(CubeMapCoordinate, GLint, const Vector2i&, std::size_t, GLvoid*);
void(CubeMapTexture::*cubeSubImage3DImplementation)(GLint, const Vector3i&, const Vector3i&, PixelFormat, PixelType, const GLvoid*, const PixelStorage&);
Expand Down
17 changes: 15 additions & 2 deletions src/Magnum/GL/Implementation/driverSpecific.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ namespace {
"arm-mali-timer-queries-oom-in-shell",
#endif

#if !defined(MAGNUM_TARGET_GLES) && defined(CORRADE_TARGET_WINDOWS)
/* ARB_direct_state_access on AMD Windows drivers has broken
glTextureSubImage3D() / glGetTextureImage() on cube map textures (but not
cube map arrays), always failing with erros like
`glTextureSubImage3D has generated an error (GL_INVALID_VALUE)` if Z size or
offset is larger than 1. Working around that by up/downloading
slice-by-slice using non-DSA APIs, similarly to the
svga3d-texture-upload-slice-by-slice workaround. The compressed image up/
download is affected as well, but we lack APIs for easy format-dependent
slicing and offset calculation, so those currently still fail. */
"amd-windows-cubemap-image3d-slice-by-slice",
#endif

#if !defined(MAGNUM_TARGET_GLES) && !defined(CORRADE_TARGET_APPLE)
/* Creating core context with specific version on AMD and NV proprietary
drivers on Linux/Windows and Intel drivers on Windows causes the context to
Expand Down Expand Up @@ -224,8 +237,8 @@ namespace {
*everything* related to cube map textures (but not cube map arrays) -- data
upload, data queries, framebuffer attachment, framebuffer copies, all
complaining about "Wrong <func> 6 provided for <target> 34067" and similar
(GL_TEXTURE_CUBE_MAP is 34067). Using the non-DSA code path as a
workaround. */
(GL_TEXTURE_CUBE_MAP is 34067). Using the non-DSA code paths as a
workaround (for the 3D image up/download as well). */
"intel-windows-broken-dsa-for-cubemaps",

/* DSA glBindTextureUnit() on Intel Windows drivers simply doesn't work when
Expand Down
Loading

0 comments on commit ca326fd

Please sign in to comment.