GUI: Also try a separate-shared-context mode #756

VelorumS · 2017-03-01T23:13:14Z

Try to init GUI as usual, fallback to the shared context mode if it fails.

TheJJ · 2017-03-02T00:07:31Z

libopenage/gui/guisys/private/platforms/context_extraction_x11.cpp

+
+		current_context = glXGetCurrentContext();
+		assert(current_context);
+	}


if this if is skipped, we should abort, right? (same above in the old function i think)

Yes, it's a good unification.

TheJJ · 2017-03-02T00:09:12Z

libopenage/gui/guisys/private/gui_event_queue_impl.cpp

+	if (QThread::currentThread() != QCoreApplication::instance()->thread())
+		this->callback_processor.processEvents();
+#else
 	this->callback_processor.processEvents();


why does apple need a special treatment here? (maybe even add a comment about the reason)

Let's leave it out and see what mac users say. Can't really test this part.

TheJJ · 2017-03-02T00:09:56Z

libopenage/gui/guisys/private/gui_ctx_setup.h

+	virtual void post_render() override;
+};
+
+class GuiSeparateRenderingContext : public CtxExtractionMode {


documentation for those classes pls

TheJJ · 2017-03-02T00:10:55Z

libopenage/gui/guisys/private/gui_ctx_setup.h

+
+namespace qtsdl {
+
+class CtxExtractionException : public std::runtime_error {


would be a good job for the error::Error so we had nice backtraces etc.

It's too deep to include the error. Replace the exception with an assert?

as you catch it yourself it should be ok like that.

Ok, that's done. They won't escape.

TheJJ · 2017-03-02T00:11:56Z

libopenage/gui/guisys/private/gui_application_impl.cpp

 	assert(std::this_thread::get_id() == this->owner);
+#ifndef __APPLE__
 	this->app.processEvents();
+#endif


comment about the reason pls, and where does apple process its events then?

TheJJ · 2017-03-02T00:13:22Z

libopenage/gui/guisys/private/gui_rendering_setup_routines.h

+
+class GuiRenderingCtxActivator;
+
+class GuiRenderingSetupRoutines {


documentation pls

TheJJ · 2017-03-02T00:14:33Z

libopenage/gui/guisys/private/opengl_debug_logger.cpp

+				switch (debugMessage.source()) {
+				case QOpenGLDebugMessage::APISource:
+					source = GL_DEBUG_SOURCE_API;
+					break;


we already have that in libopenage/error/gl_debug.cpp, we should not duplicate it. any idea how we could unify them?

Can remove the switch.

TheJJ · 2017-03-04T11:56:18Z

libopenage/gui/guisys/private/opengl_debug_logger.cpp

+
+void apply_opengl_debug_parameters(gl_debug_parameters params, QOpenGLContext &current_dest_context)  {
+#ifndef __APPLE__
+	if (params.is_debug && params.callback) {


can you elaborate a bit why apple needs this special treatment of debug function config? I want to understand/

Should have cleaned this up like in that other place.

AFAIK, apple doesn't support this GL version, so there may be some problems. Maybe Qt can deal with them by itself without the ifdefs.

TheJJ · 2017-03-04T19:44:19Z

Simple question: what is the separate-shared-context mode? 😁 Is it to have the gui opengl context separately?

VelorumS · 2017-03-04T19:47:50Z

Yes, that creates another context just for gui.

TheJJ · 2017-03-08T12:21:37Z

libopenage/gui/guisys/private/gui_rendering_setup_routines.cpp

+		try {
+			this->ctx_extraction_mode = std::make_unique<GuiSeparateRenderingContext>(window);
+		} catch (const CtxExtractionException&) {
+			assert(false && "setting up context for GUI failed");


probably a debugging leftover

The whole assert or the comment in it?

That's what I called "replace the exception with an assert".

It's kind of a spiritual successor to this one:

openage/libopenage/gui/guisys/private/platforms/context_extraction_x11.cpp

Line 26 in 816fa62

assert(current_context);

hm, i was just confused by the false &&, but it seems alright :)

TheJJ · 2017-03-08T12:26:12Z

libopenage/gui/guisys/private/platforms/context_extraction_cocoa.mm

@@ -1,4 +1,4 @@
-// Copyright 2015-2016 the openage authors. See copying.md for legal info.
+// Copyright 2015-2017 the openage authors. See copying.md for legal info.


btw why does the cocoa extraction have to be a .mm file?

That's a valid question. With current approach there is probably no need to do ObjC there.

Do you wanna fix that? Would be good :)

TheJJ · 2017-03-08T12:26:22Z

libopenage/gui/guisys/private/platforms/context_extraction_x11.cpp

+			QVariant::fromValue<QGLXNativeContext>(
+				QGLXNativeContext(current_context,
+								  wm_info.info.x11.display,
+								  wm_info.info.x11.window)),


Birch-san · 2017-04-08T23:29:00Z

I tried to compile this on latest Sierra, but there were some missing #defines. To fix this: I provide a pull request for your pull request:

VelorumS#2

Birch-san · 2017-04-08T23:34:08Z

Good news

launches (with my pull request):

➜  openage git:(gui-gl-context) make run
[  1%] Automatic moc for target libopenage
[  1%] compiling .py files to .pyc files
[1/1] Compiling /Users/anon/git/openage/openage/convert/hdlanguagefile.py to /Users/anon/git/openage/openage/convert/__pycache__/hdlanguagefile.cpython-36.pyc
./run game
INFO [py] launching openage v0.3.0-861-g85e62658
INFO [py] compiled by AppleClang 8.0.0.8000042
INFO [py] running in DEVMODE
INFO launching engine with Path(Union(Directory(/Users/anon/git/openage/cfg).root @ (b'cfg',), Directory(/Users/anon/git/openage/assets).root @ (b'assets',)):) and fps limit 0
INFO SDL audio subsystems initialized
INFO Using audio device: default [freq=48000, format=32784, channels=2, samples=4096]
INFO Initialized SDL video subsystems.
INFO Compiled with Qt 5.8.0 and run with Qt 5.8.0
INFO Falling back to separate render context for GUI
INFO loading configuration files...
INFO [py] loading config file [Union(Directory(/Users/anon/git/openage/cfg).root @ (b'cfg',), Directory(/Users/anon/git/openage/assets).root @ (b'assets',))]:cfg/keybinds.oac...
INFO Loading time [engine]: 0.531 s
INFO Loading time   [game]: 0.613 s
INFO [T1] Loading game specification files...
INFO [T1] Loaded multi-csv file: 99984 sub-files
INFO [T1] Loading textures...
INFO [T1] Loading sounds...
INFO [T1] Loading time  [data]: 19.449 s

And it doesn't do the infinite stack trace thing. CPU usage is 0%, so that's good.

Bad news

Can't kill it with Ctrl-C.

Nothing renders, ever. Nor are any sounds made.

Birch-san · 2017-04-08T23:43:48Z

is there a way to run openage with placeholder assets? since maybe the black screen just indicates that my assets weren't extracted correctly. admittedly my assets/converted/graphics looks legit, but still it would be nice to rule it out as a problem.

TheJJ · 2017-04-09T09:47:19Z

Unfortunately not, if the log doesn't say anything about load-failures they should be loaded correctly. I guess that the separate gui context fails to be displayed.

VelorumS · 2017-04-09T17:06:12Z

@Birch-san, we also discussed in this PR some changes to the input handling (in outdated comments): libopenage/gui/guisys/private/gui_application_impl.cpp and libopenage/gui/guisys/private/gui_event_queue_impl.cpp.

Dropped that commit because it was unexplainable. I've put it here:

VelorumS@d577f56

Birch-san · 2017-04-09T18:01:27Z

that commit d577f56 certainly helps!

Birch-san · 2017-04-09T18:14:14Z

Encountered these errors whilst using the game (looks like it's just complaining about audio format):

https://gist.github.com/Birch-san/cc9220818094b61cc9b817925da1e036

Birch-san · 2017-04-09T18:18:09Z

Here's a gameplay video.

https://youtu.be/9GmwKreWg6I

Colours are off in a strange way. I wonder if it's an endianness problem? I saw color shifts like this once when compiling a Game Boy Advance emulator for Mac, and it was totally endianness.

I wasn't able to interact with anything in the game, but the HUD buttons seemed to be clickable (though they spent a lot of their time disappearing).

Birch-san · 2017-04-09T18:22:59Z

The exact code I was running is here:
Birch-san@e83a4e3

Which (at the time of writing) is the HEAD of this branch:
https://github.com/Birch-san/openage/tree/gui-gl-context-fix-3

VelorumS · 2017-04-09T18:52:58Z

A theory about the disappearing buttons: it's probably related to that input handling fix - some events can't pass through if there is not enough real input activity (so the GUI isn't updated).

The debug overlay (the one with the graph) has correct colors. The GUI too.

do the asset png files look correctly?
is the ingame HUD ok? (the one with the population and resource count)

You can't interact because the game isn't in the Action mode. Press M several times or the 'change mode' button.

Birch-san · 2017-04-09T20:33:47Z

oh, I was able to see the HUD by clicking "change mode" a few times. it's a bit damaged.

Birch-san · 2017-04-09T20:34:34Z

when I changed into a different mode, I was able to select units:

Birch-san · 2017-04-09T20:36:05Z

action mode:

Birch-san · 2017-04-09T20:37:24Z

some events can't pass through if there is not enough real input activity (so the GUI isn't updated).

certainly I found that sometimes I had to click buttons several times before they would work.

Birch-san · 2017-04-14T00:37:33Z

I fixed the color problem. I was able to confirm in Photoshop that it was just a case of "red & blue channels' being swapped". I was able to fix the pixel output format.

See my pull request:

VelorumS#3

TheJJ · 2017-04-22T13:12:06Z

@ChipmunkV Do you still want to get rid of the objective C file?

VelorumS · 2017-04-22T17:07:16Z

@TheJJ, yes, but I only see how to remove a part of it.

VelorumS · 2017-04-29T09:40:41Z

Added missing GL_DEBUG definitions to fix compile on Mac and minimized the ObjC file to just getting the current context and putting it into Qt.

TheJJ

coal!

VelorumS changed the title ~~GUI: Also try a separate-context mode~~ GUI: Also try a separate-shared-context mode Mar 1, 2017

VelorumS force-pushed the gui-gl-context branch from b02cd3e to c055843 Compare March 1, 2017 23:16

TheJJ reviewed Mar 2, 2017

View reviewed changes

TheJJ added lang: c++ Done in C++ code improvement Enhancement of an existing component area: ui Related to the graphical user interface (Qt) labels Mar 2, 2017

VelorumS force-pushed the gui-gl-context branch 2 times, most recently from 25733ef to f66d9e8 Compare March 3, 2017 21:30

TheJJ reviewed Mar 4, 2017

View reviewed changes

VelorumS force-pushed the gui-gl-context branch from f66d9e8 to 1ba6daf Compare March 4, 2017 14:06

TheJJ reviewed Mar 8, 2017

View reviewed changes

VelorumS force-pushed the gui-gl-context branch from 1ba6daf to b9d15b7 Compare March 21, 2017 20:39

VelorumS mentioned this pull request Mar 21, 2017

make compiler issues on os x sierra #768

Closed

Birch-san mentioned this pull request Apr 8, 2017

"unrecognized selector sent to instance" when running on Mac #784

Closed

VelorumS and others added 7 commits April 29, 2017 10:06

fix include

6b1e464

guisys: separate context mode

e769b60

guisys: logger for separate context mode

ff1b7ff

guisys: context init sequences for unique and separate modes

76b9a97

guisys: common interface for context init modes

2b90c96

guisys: try both init modes

3ea6df9

provide missing GL_DEBUG definitions to fix compile on Mac

2bc8a97

VelorumS force-pushed the gui-gl-context branch from b9d15b7 to 2bc8a97 Compare April 29, 2017 09:32

TheJJ approved these changes Apr 30, 2017

View reviewed changes

TheJJ merged commit cc76276 into SFTtech:master Apr 30, 2017

VelorumS mentioned this pull request May 2, 2017

make compiler issues on os x Yosemite. #819

Closed

Birch-san mentioned this pull request May 2, 2017

Game crashes on launch (macOS) #822

Closed

Birch-san mentioned this pull request May 29, 2017

macOS event dispatching broken #852

Closed


		namespace qtsdl {

		class CtxExtractionException : public std::runtime_error {


		class GuiRenderingCtxActivator;

		class GuiRenderingSetupRoutines {

		@@ -1,4 +1,4 @@
		// Copyright 2015-2016 the openage authors. See copying.md for legal info.
		// Copyright 2015-2017 the openage authors. See copying.md for legal info.

Uh oh!

GUI: Also try a separate-shared-context mode #756

GUI: Also try a separate-shared-context mode #756

Uh oh!

Conversation

VelorumS commented Mar 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheJJ Mar 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheJJ commented Mar 4, 2017

Uh oh!

VelorumS commented Mar 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Birch-san commented Apr 8, 2017

Uh oh!

Birch-san commented Apr 8, 2017

Uh oh!

Birch-san commented Apr 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheJJ commented Apr 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VelorumS commented Apr 9, 2017

Uh oh!

Birch-san commented Apr 9, 2017

Uh oh!

Birch-san commented Apr 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Birch-san commented Apr 9, 2017

Uh oh!

Birch-san commented Apr 9, 2017

Uh oh!

VelorumS commented Apr 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TheJJ Mar 4, 2017 •

edited

Loading

Birch-san commented Apr 8, 2017 •

edited

Loading

TheJJ commented Apr 9, 2017 •

edited

Loading

Birch-san commented Apr 9, 2017 •

edited

Loading

VelorumS commented Apr 9, 2017 •

edited

Loading