Skip to content
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

State has wrong behavior #329

Open
X-Ryl669 opened this issue Dec 12, 2016 · 2 comments
Open

State has wrong behavior #329

X-Ryl669 opened this issue Dec 12, 2016 · 2 comments

Comments

@X-Ryl669
Copy link

X-Ryl669 commented Dec 12, 2016

This code:

auto currentState = globjects::State::currentState();
[...] // No state related code here

// Then change some state
globjects::ref_ptr<globjects::State> state = new globjects::State(globjects::State::ImmediateMode); // Apply immediately all changes
state->pixelStore(gl::GL_UNPACK_ALIGNMENT, 1); // In components size
state->pixelStore(gl::GL_UNPACK_ROW_LENGTH, width/3); // In components size

[...]
currentState->apply();

Trigger this GL error:

error: 0x500, high severity (API)
        GL_INVALID_ENUM error generated. Polygon modes for <face> are disabled in the current profile.

The backtrace for this error is:

#0  Stitcher::Application::__lambda0::operator() (__closure=0x10ed440, message=...) at ../src/Application.cpp:478
#1  0x00000000005b3f6d in std::_Function_handler<void(const globjects::DebugMessage&), Stitcher::Application::initSystem()::__lambda0>::_M_invoke(const std::_Any_data &, const globjects::DebugMessage &) (__functor=..., __args#0=...)
    at /usr/include/c++/4.8.2/functional:2071
#2  0x0000000000623649 in globjects::DebugImplementation_DebugKHR::debugMessageCallback(gl::GLenum, gl::GLenum, unsigned int, gl::GLenum, int, char const*, void const*) ()
#3  0x00007fffece62b18 in ?? () from /lib64/libnvidia-glcore.so.367.57
#4  0x00007fffece62c60 in ?? () from /lib64/libnvidia-glcore.so.367.57
#5  0x00007fffece62f22 in ?? () from /lib64/libnvidia-glcore.so.367.57
#6  0x000000000068e1c2 in glbinding::Function<void, gl::GLenum, gl::GLenum>::call(gl::GLenum&, gl::GLenum&) const ()
#7  0x00000000007383e2 in gl::glPolygonMode(gl::GLenum, gl::GLenum) ()
#8  0x000000000062c851 in globjects::State::apply() ()

Clearly, it's dealing with polygon mode which I've not touched in my code.
I wonder if the "currentState" is a good idea at all. It's slow (because it loops other all states to restore them). I wonder if a per-thread storage list of modified state wouldn't be better. That is, any state changed with a State instance would append the previous state to a TLS's list (if it's not found in the list beforehand, obviously), and a static restoreState() would pop all item from the TLS's list to restore all the current modified state and not all the states like it's done currently, thus avoiding the error written above.

@scheibel
Copy link
Member

You're right, a fine-grained handling of states would be best. We actually had plans to create state-stacks from which you can push and pop that would compute the actual difference for restoring the state (#149).
However, to compute differences we have to know the ground-truth and that is what globjects::State::currentState is about.

I assume you use a core profile, so polygon modes are disabled?

@X-Ryl669
Copy link
Author

However, to compute differences we have to know the ground-truth and that is what globjects::State::currentState is about.

Not necessarly. It depends where you want to pay the cost for state tracking. Either you pay the cost on first use (that is, you need to search for a state change in your state's stack and if not found, you'll have to query the driver for the "current" state's value), or on initialization (like the currentState does). For the fastest state changing, the latter is best, since you bootstrap from a known state (and as such, state change will be 1:1 to what's done with standard gl-func based coding). The former is more intuitive through, since you don't care about having to deal with a currentState anymore.

In general case however, if you expect some state, you should set it (and not rely on whatever driver's default value that could be modified by any library used), so maybe an hybrid approach would be better where you can create a "default" state stack value, you can restore anytime. This would avoid the need to searching the previous state every time you change a state.

Yes, I'm using a core profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants