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

WIP: Fix for Shaders and Switch to QOpenGL #1308

Closed
wants to merge 6 commits into from
Closed

WIP: Fix for Shaders and Switch to QOpenGL #1308

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 1, 2017

This is still pretty buggy, but shaders don't instantly crash OT anymore. It works, but throws a lot of assert errors.

shaders

@shun-iwasawa I have no experience doing OpenGL programming and would love some help.

@ghost ghost mentioned this pull request Jul 1, 2017
@ghost
Copy link
Author

ghost commented Jul 2, 2017

So here's the limit of what I can find:

I keep getting a "cannot send events to objects owned by a different thread" error around lines that contain:

m_imp->m_context->create();
m_imp->m_context->makeCurrent(m_imp->m_surface);

Ignoring this doesn't crash the program. I have tried a variety of fixes but my limited knowledge of threading and/or opengl isn't able to fix the problem.

There is also a problem in the destructor of sceneviewer when trying to clear out l_proxy and l_contexts.

I am going to stop here and hope someone is able/willing to pick up the pieces since I seem to be at a dead end.

@ghost
Copy link
Author

ghost commented Jul 30, 2017

@blackwarthog - would you have any time to take a look at this? Having usable shaders is so close, but I can't figure out the last bit.

@blackwarthog
Copy link
Collaborator

At my system I've seen only warnings about wrong thread of context.

Seems you was at the right way, i just removed two other MACOSX preprocessor condiditions to call moveToThread in ShadingContext destructor and in ShadingContext::doneCurrent too:

blackwarthog@1213cfb

And I've no more warnings about context thread. I've checked it linux, please, tell me how it works at windows.

Also at windows and linux uses OpenGL 2.0 (by default) but for MacOS uses version 3.2. If some shaders requires OpenGL 3, then we may to select OpenGL 3.2 for all platforms (in function ShadingContext::Imp::format()).

@ghost
Copy link
Author

ghost commented Aug 7, 2017

With the new changes I get an instant crash, no assert errors or anything. It doesn't even trigger Visual Studio where it crashed. It just goes down hard.

@blackwarthog
Copy link
Collaborator

ok, i'll try to check it on Windows.

@blackwarthog
Copy link
Collaborator

I cannot test MSVC build at PC with graphics accelerator. I've tested MinGW build and it works at Win7 64 and GeForce 9600GT. You may to download it from from here: https://dev.icystar.com/downloads/OpenToonz-1.1.3-morevna-2017.08.08-3ea6d-win-64bit.exe

@blackwarthog
Copy link
Collaborator

May be latest changes in master affects shaders behavior. You may try to do rebase to latest master.

@ghost
Copy link
Author

ghost commented Aug 8, 2017

@blackwarthog I checked your build and it works on the first 6 shaders, but the blur ones cause an instant crash still. I did a build rebased with master and added your changes and if I run a release build, it works, but if I run a debug build, shader preview instantly takes OT down. I'm not sure why debug would be different than release.

Thanks for your help with this. I really appreciate it.

@ghost ghost mentioned this pull request Sep 7, 2017
@shun-iwasawa
Copy link
Member

shun-iwasawa commented Oct 4, 2017

I still do not find the way to fix the problem above, but confirmed that this PR as-is will fix the issue #1081 (crash with full screen mode on OSX) .

@artisteacher
Copy link
Contributor

If this could fix all the fullscreen crashing, including #1396, that would be so amazing!

@ghost
Copy link
Author

ghost commented Oct 18, 2017

Closing - Updated in #1517

@ghost ghost closed this Oct 18, 2017
This pull request was closed.
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.

None yet

3 participants