Skip to content

Intel Mesa OpenGL bug fixes and cleanup #368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 10, 2015
Merged

Intel Mesa OpenGL bug fixes and cleanup #368

merged 8 commits into from
Sep 10, 2015

Conversation

xlz
Copy link
Member

@xlz xlz commented Aug 22, 2015

This is rebased on #364. Commits of this PR begin from "changed minimal opengl version to 3.1".

This PR fixes all bugs on Intel GPU. This PR supersedes #352.

Performance comparison on i7-4600U @ 2.10GHz:

[Info] [OpenGLDepthPacketProcessor] avg. time: 21.3259ms -> ~46.8913Hz
[Info] [OpenCLDepthPacketProcessor] avg. time: 16.9965ms -> ~58.8356Hz

@@ -347,10 +424,10 @@ struct OpenGLDepthPacketProcessorImpl : public WithOpenGLBindings
Texture<U16C1> input_data;

Texture<F32C4> stage1_debug;
Texture<F32C3> stage1_data[3];
Texture<F32C4> stage1_data[3];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what was the particular effect of the old value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect is the texture can't be attached to FBO. With INTEL_DEBUG=fbo,... you get FBO incomplete: Unsupported HW texture/renderbuffer format attached: MESA_FORMAT_RGB_FLOAT32

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original diagnosis was in #352.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, thanks for the explanation

@xlz
Copy link
Member Author

xlz commented Aug 31, 2015

v2:

  • ocl-icd bug detection can only happen in run-time, but the workaround requires compile-time change. So all Linux builds now disable exceptions. ocl-icd bug detection just prints a warning.
  • Prepend #define MESA_BUGGY_BOOL_CMP if the Mesa bug is detected.
  • checkMesaVersion() is replaced by prepending definitions.

@xlz
Copy link
Member Author

xlz commented Sep 1, 2015

v3:

  • Moved Mesa bug detection out from ShaderProgram to runtime methods, because glGetString will return NULL in the constructor.

@floe floe added this to the 0.1 milestone Sep 2, 2015
@xlz
Copy link
Member Author

xlz commented Sep 3, 2015

Rebased to master.

@floe
Copy link
Contributor

floe commented Sep 4, 2015

Could you please rebase once more, so it only shows the new commits?

@floe
Copy link
Contributor

floe commented Sep 4, 2015

Tested on Ubuntu 14.04 with Intel Corporation 4th Gen Core Processor Integrated Graphics Controller (rev 06), works like a charm (and actually 10 Hz faster than OpenCL). Kudos.

christiankerl and others added 8 commits September 4, 2015 10:07
Intel/Mesa has GL_MAX_RECTANGLE_TEXTURE_SIZE=4096, but this was
asking for 424*10.

Drop the 10th frame which seems useless now, so the texture size
works for Intel/Mesa.
F32C3 format is not supported on Intel/Mesa making FBOs incomplete.

Just change F32C3 to F32C4, and vec3 output automatically expands
to vec4.
Also add completeness checks to each FBO.
Mesa 10.2.9 and older versions are oblivious to a behavior change
in the CMP instruction on Intel CPU SandyBridge and newer.

On SandyBridge and newer ones, CMP instruction sets all bits to one
in dst register (-1) as boolean true value. Before that, only the
LSB is set to one with other bits being undefined.

Mesa 10.2.9 and older use XOR instruction on the LSB for the logical
not operator, which produces -2 as boolean value for !true.
The value is then used by SEL instruction in mix(), which compares
the value with zero and does not clear high bits before that,
selecting wrong components.

A macro MESA_BUGGY_BOOL_CMP is added to forcibly convert -1 to 1
for Mesa 10.2.9 and older before logical not result is used for
mix(). The rest of comparison operators and conditionals are safe
from this behavior.

I could not independently reproduce this behavior in a seperate
standalone problem. It is possibly because instruction generation
varies from optimization.

This behavior was fixed in Mesa upstream
2e51dc838be177a09f60958da7d1d904f1038d9c, only appearing in 10.3+.
Remove commented definitions. They can be found in commit history.

Move OpenGL version check out of flextGL, and use LOG_* macros
for error reporting.
Assembly errors and lost packets should not flood the log output.
@xlz
Copy link
Member Author

xlz commented Sep 4, 2015

Rebased to master.

@xlz
Copy link
Member Author

xlz commented Sep 4, 2015

actually 10 Hz faster than OpenCL

This is weird. Did you use ./Protonect -noviewer for benchmarking? OpenGL should be always slower than OpenCL right now.

@floe
Copy link
Contributor

floe commented Sep 6, 2015

No, this was with the viewer enabled. Nevertheless, since the viewer and the OpenGL DPP are not linked together, I would assume that download and re-upload to GPU memory still have to happen?

@xlz
Copy link
Member Author

xlz commented Sep 7, 2015

Yes. OpenGL should be slower because it does extra flipping on CPU.

@floe
Copy link
Contributor

floe commented Sep 10, 2015

As usual: if we don't merge it, we'll never know if it has hidden issues. Merging.

floe added a commit that referenced this pull request Sep 10, 2015
Intel Mesa OpenGL bug fixes and cleanup
@floe floe merged commit d855d25 into OpenKinect:master Sep 10, 2015
@xlz xlz deleted the intel-opengl branch September 13, 2015 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants