Skip to content

Commit

Permalink
igl | opengl | Repeat glFramebufferTexture2DMultisample with GL_FRAME…
Browse files Browse the repository at this point in the history
…BUFFER (instead of GL_DRAW/READ_FRAMEBUFFER) to mitigate for driver behavior.

Summary:
We are seeing driver not following spec, where if used specifically with GL_DRAW/READ_FRAMEBUFFER - fails, but with GL_FRAMEBUFFER - it's all fine.
Just plug the hole within the IContext implementation, and repeat the last command with GL_FRAMEBUFFER, so we still can get the feature nicely.

Reviewed By: dmannemela

Differential Revision: D49675850

fbshipit-source-id: b772665d9d1798cf6a29202952ad2484f9e2e1c1
  • Loading branch information
nlutsenko authored and facebook-github-bot committed Sep 27, 2023
1 parent 7d81394 commit a2d626a
Showing 1 changed file with 27 additions and 9 deletions.
36 changes: 27 additions & 9 deletions src/igl/opengl/IContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,20 @@ void logSource(const int count, const char** string, const int* length) {
#if IGL_DEBUG
#define GLCHECK_ERRORS() \
do { \
if (alwaysCheckError_) \
if (alwaysCheckError_) { \
checkForErrors(__FUNCTION__, __LINE__); \
} \
} while (false)
#define GL_ASSERT_ERROR(condition, callerName, lineNum, errorCode) \
IGL_ASSERT_MSG((condition), \
"[IGL] OpenGL error [%s:%zu] 0x%04X: %s\n", \
callerName, \
lineNum, \
errorCode, \
GL_ERROR_TO_STRING(errorCode))
#else
#define GLCHECK_ERRORS()
#define GLCHECK_ERRORS() static_cast<void>(0)
#define GL_ASSERT_ERROR(condition, callerName, lineNum, errorCode) static_cast<void>(0)
#endif // IGL_DEBUG

#define RESULT_CASE(res) \
Expand Down Expand Up @@ -1511,7 +1520,21 @@ void IContext::framebufferTexture2DMultisample(GLenum target,
texture,
level,
samples);
GLCHECK_ERRORS();

// Certain drivers (I am looking at you Adreno) misbehave according to spec,
// where GL_DRAW_FRAMEBUFFER is perfectly valid framebuffer target, but it only recognizes
// GL_FRAMEBUFFER meaning that it behaves as if it is `IMG_multisampled_render_to_texture` not
// `EXT_multisampled_render_to_texture`.
auto error = getError();
if (error == GL_INVALID_ENUM &&
(target == GL_DRAW_FRAMEBUFFER || target == GL_READ_FRAMEBUFFER) &&
deviceFeatureSet_.hasExtension(Extensions::MultiSampleExt)) {
// Repeat again, now using explicitly GL_FRAMEBUFFER not GL_DRAW/GL_READ.
framebufferTexture2DMultisample(GL_FRAMEBUFFER, attachment, textarget, texture, level, samples);
} else if (alwaysCheckError_) {
lastError_ = error;
GL_ASSERT_ERROR(lastError_ == GL_NO_ERROR, __FUNCTION__, __LINE__, error);
}
}

void IContext::framebufferTextureLayer(GLenum target,
Expand Down Expand Up @@ -3232,12 +3255,7 @@ GLenum IContext::checkForErrors(IGL_MAYBE_UNUSED const char* callerName,
IGL_MAYBE_UNUSED size_t lineNum) const {
lastError_ = getError();

IGL_ASSERT_MSG(lastError_ == GL_NO_ERROR,
"[IGL] OpenGL error [%s:%zu] 0x%04X: %s\n",
callerName,
lineNum,
lastError_,
GL_ERROR_TO_STRING(lastError_));
GL_ASSERT_ERROR(lastError_ == GL_NO_ERROR, callerName, lineNum, lastError_);

return lastError_;
}
Expand Down

0 comments on commit a2d626a

Please sign in to comment.