feat(reactor): make separate graphic scheduler/thread #4865

DarkWeird · 2021-08-24T08:42:02Z

Contains

Introducing GRAPHICS Scheduler
wraps calls to GL/GLFW to runBlocking at graphics scheduler ( hopping flow to another thread)
Splash screen should works at MacOS now. (ref: Follow-ups after LWJGL3 #4206)
splitting opengl context between splashscreen and game
wrapped calls to runBlocking should tracking by ActivityMonitor
Ref: Add specific macos flags for window creating SplashScreen#7 has macos specific flags for macos (should helps with macos glitches like red window, wring dpi)

How to test

runs Terasology with splash screen (especial at MacOs)
start any gameplay
try to call maximum rendering stuffs
3.1. block digging particles
3.2. different Overlay renderer.

Outstanding before merging

docs
refactor
cleanup

After merge:

rethink/refactor reactor usage with LWJGL subsystem, lwjgl's assets and opengl rendering.

* splash screen's loop handles by Flux/Mono * Most calls to GL/GLFW runs at Graphic Thread via GameScheduler#runBlockingGraphics.

…gerSubsystem' into refactor/remove-usage-ThreadManagerSubsystem

* Some Docs

DarkWeird · 2021-08-24T12:34:37Z

TeraED was broken before this PR :(
seems splashscreen flag was broken at Pico-cli PR (you cannot disable splashscreen)

* remove unused code

skaldarnar · 2021-08-24T18:27:00Z

I did run gradlew game on Mac OS with this PR. The splash screen is not rendering correctly (it's a flashing, half-transparent red rectangle on screen), but the game starts up fine afterwards.

I could not really play it as I was at 0.6 fps, but that might be related to too much load on my machine...

TeraED was broken before this PR :(

What do you mean by that? I haven't tried to start TeraED for ages, but it at least compiled just fine when running gradlew jar for the whole workspace 🤔

DarkWeird · 2021-08-24T18:36:00Z

What do you mean by that? I haven't tried to start TeraED for ages, but it at least compiled just fine when running gradlew jar for the whole workspace 🤔

TeraED hardcrashes with many nullpointers during initializing MainMenuState. This errors not related with rendering code.

I could not really play it as I was at 0.6 fps, but that might be related to too much load on my machine...

Hmm. Which gpu are you using?
Seems macbooks have 2 gpus(or not)
What about without splash screen?(flag --no-splash not works after picocli(seems))
And how without this pr?

The splash screen is not rendering correctly (it's a flashing, half-transparent red rectangle on screen), but the game starts up fine afterwards.

Sounds familiar. With red rectangle. I will google it.

DarkWeird · 2021-08-24T18:50:07Z

Ok. Splashscreen Rendering issues at macos should be resolved at splashscreen repo.
(I will create issue tomorrow)

keturn

this is an interesting idea, but seeing it in practice is scary

I think a lot of what I'm seeing is a byproduct of the fact that this is wrapping a lot of stuff in the runBlockingGraphics scheduler to keep it as close as possible to the existing implementation, which is arguably the right way to get something like this done in an incremental way.

but so much blocking
and so much confusion about "do I need to wrap this in runGraphics, or is the method I'm calling going to do that?"

keturn · 2021-08-27T23:56:38Z

engine/src/main/java/org/terasology/engine/core/GameScheduler.java

+    public static <T> Flux<T> wrapActivity(Scheduler scheduler, String name, Flux<T> mono) {
+        return Flux.using(
+                () -> ThreadMonitor.startThreadActivity(name),
+                activity -> mono,
+                ThreadActivity::close


I convinced myself this works for a Runnable turned in to a Mono, because we know the Runnable will execute and be on that thread until it is done, and then the mono is over.

Flux is less clear. There could be lots of time between events. Do we report that as all one thread activity?

also, other stuff could be running on that thread between the events of that Flux, is that going to mess up the assumptions in the ThreadMonitor implementation?

Nice catch.
Needs to handle both cases.
Whole flux as thread activity and element of flux as thread activity.

Tasks that running at graphic scheduler works via fifo queue (see threadcapturedscheduler)
Then no one switching during processing.

keturn · 2021-08-27T23:57:24Z

engine/src/main/java/org/terasology/engine/core/GameScheduler.java

+    }
+
+    /**
+     * Wraps {@link Mono} with activity monitor.


keturn · 2021-08-28T00:01:56Z

engine/src/main/java/org/terasology/engine/core/GameScheduler.java

+        if (!graphicsScheduler.isSchedulerThread(Thread.currentThread())) {
+            mono = mono
+                    .doOnSubscribe(s-> GLFW.glfwMakeContextCurrent(LwjglGraphics.windowId))
+                    .doFinally(f -> GLFW.glfwMakeContextCurrent(MemoryUtil.NULL))
+                    .subscribeOn(graphicsScheduler);


I'm not experienced with multithreaded use of GL, so I Am Not An Expert, but I've read like 1.2 papers on it, and I am a little worried about making much use of glfwMakeContextCurrent. I hear the context-switching is generally not worth it.

There required for handling multiple contexts/windows.

At linux and windows you can use separate threads for handling separate gl contexts (like window/cintext per thread)
At macos - not.

Then you should change context i this way.

Splashscreen... (You can see some screenshots with messing gl contexts at #architecture)

keturn · 2021-08-28T00:07:56Z

engine/src/main/java/org/terasology/engine/core/GameScheduler.java

+                    .doFinally(f -> GLFW.glfwMakeContextCurrent(MemoryUtil.NULL))
+                    .subscribeOn(graphicsScheduler);
+        }
+        return mono.block();


Do we need a blocking API for this? I didn't think GL calls were synchronous or had return values.

Some graphics monos load assets and may corrupt state(concurrent access) in theory.

Another. This places was design and tested to run in sequence

I choose fail safe path then for now

keturn · 2021-08-28T00:15:13Z

engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglDisplayDevice.java

+        return GameScheduler.runBlockingGraphics("hasFocus",
+                () -> GLFW.GLFW_TRUE == GLFW.glfwGetWindowAttrib(GLFW.glfwGetCurrentContext(), GLFW.GLFW_FOCUSED));


oh, here's that blocking use case I was asking about.

bother, this API is a mashup of things, as some of them are about the GL context, and other things (like focus) are not part of the GL API at all.

Overall, I don't think these methods that wrap a single glfw function are worth using GameScheduler.wrapActivity on. They shouldn't take enough time to be worth clocking, and, well, they block the graphics thread, so we're not going to be seeing those as open tasks on any monitor that renders via the graphics thread.

Glfw related to context too.

Gl context = glfw window handle.

Rules about gl context at one thread applicable for glfw partially (and there i takes context from static. There needs a refactoring to stores context/window handle and reuse it)

keturn · 2021-08-28T00:40:29Z

engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglDisplayDevice.java

+        GameScheduler.runBlockingGraphics("isFullscreen", () ->
+                glViewport(0, 0, width, height)


cut/paste error in description

keturn · 2021-08-28T00:47:33Z

engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglGraphicsManager.java

@@ -46,16 +42,9 @@
 import static org.lwjgl.opengl.GL11.glGenTextures;
 import static org.lwjgl.opengl.GL11.glTexParameterf;

-public class LwjglGraphicsManager implements LwjglGraphicsProcessing {
-
-    private final BlockingDeque<Runnable> displayThreadActions = Queues.newLinkedBlockingDeque();


We already had a queue of display thread actions? HMM.

See main of terasology.
And ThreadCapturedScheduler.

keturn · 2021-08-28T00:50:17Z

engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglGraphicsManager.java

-        asynchToDisplayThread(() -> {
-            int id = glGenTextures();
-            reloadTexture2D(id, buffers, wrapMode, filterMode, width, height);
-            idConsumer.accept(id);
-        });
+        int id = glGenTextures();
+        reloadTexture2D(id, buffers, wrapMode, filterMode, width, height);
+        idConsumer.accept(id);


Some of these methods lost their async wrappers? why did createTexture3D keep it but not this createTexture2D?

I don't think about this methods like this.

Textures3d runs in graphics now (doReload)
Textures2d do same already.

Needs to revisit this place later

keturn · 2021-08-28T01:04:52Z

facades/PC/src/main/java/org/terasology/engine/GLFWSplashScreen.java

+        Renderer renderer = Mono.just(0)
+                .subscribeOn(GameScheduler.graphics())
+                .doOnNext(n -> initWindow())
+                .map((n) -> new Renderer())
+                .doOnNext(Renderer::init)


Starting this with just(0) instead of initWindow feels weird. Is that so you have something to subscribeOn? Is this the sort of thing that publishOn is for?

I detected mono.fromRunable method too late.

All methods used there should runs at graphics.

facades/PC/src/main/java/org/terasology/engine/GLFWSplashScreen.java

…t/add-graphics-scheduler

DarkWeird · 2021-09-13T10:25:28Z

@skaldarnar
recheck at macos plz.
enabled vsync - should helps a bit.

# Conflicts: # engine/src/main/java/org/terasology/engine/rendering/primitives/ChunkMesh.java

…scheduler # Conflicts: # engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglDisplayDevice.java # engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglGraphics.java

skaldarnar · 2022-03-23T17:43:39Z

Now I get the following error about my Mac not being able to support the required OpenGL version (I think that's what '330' refers to, right?)... 😕 It should be able to meet this requirement, though (MacBook Pro 15, 2016), listed with support for OpenGL v4.1 on Apple's support site.

java.lang.RuntimeException: ERROR: 0:1: '' :  version '330' is not supported
ERROR: 0:1: '' : syntax error: #version

        at org.terasology.splash.glfw.graphics.Shader.checkStatus(Shader.java:87)
        at org.terasology.splash.glfw.graphics.Shader.compile(Shader.java:78)
        at org.terasology.splash.glfw.graphics.Shader.createShader(Shader.java:119)
        at org.terasology.splash.glfw.graphics.Shader.loadShader(Shader.java:146)
        at org.terasology.splash.glfw.graphics.Renderer.setupShaderProgram(Renderer.java:407)
        at org.terasology.splash.glfw.graphics.Renderer.init(Renderer.java:77)
        at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:185)
        at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:120)
        at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:200)
        at reactor.core.publisher.FluxSubscribeOnValue$ScheduledScalar.run(FluxSubscribeOnValue.java:180)
        at org.terasology.engine.core.schedulers.ThreadCaptureScheduler.start(ThreadCaptureScheduler.java:59)
        at org.terasology.engine.Terasology.main(Terasology.java:157)
        Suppressed: java.lang.Exception: #block terminated with an error
                at reactor.core.publisher.BlockingSingleSubscriber.blockingGet(BlockingSingleSubscriber.java:99)
                at reactor.core.publisher.Mono.block(Mono.java:1706)
                at org.terasology.engine.GLFWSplashScreen.run(GLFWSplashScreen.java:64)
                at org.terasology.engine.Terasology.call(Terasology.java:168)
                at org.terasology.engine.Terasology.call(Terasology.java:68)
                at picocli.CommandLine.executeUserObject(CommandLine.java:1933)
                at picocli.CommandLine.access$1200(CommandLine.java:145)
                at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2332)
                at picocli.CommandLine$RunLast.handle(CommandLine.java:2326)
                at picocli.CommandLine$RunLast.handle(CommandLine.java:2291)
                at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2159)
                at picocli.CommandLine.execute(CommandLine.java:2058)
                at org.terasology.engine.Terasology.lambda$main$0(Terasology.java:146)
                at java.base/java.lang.Thread.run(Thread.java:829)

However, glxinfo also just shows "2.1 ATI-4.7.103" while picking up the Radeon GPU...

OpenGL vendor string: ATI Technologies Inc.
OpenGL renderer string: AMD Radeon Pro 455 OpenGL Engine
OpenGL version string: 2.1 ATI-4.7.103
OpenGL shading language version string: 1.20

Edit: Seems related to MC-153714, siumilar error and similar/same detected version

This question on StackOverflow suggests to replace your code from #include <gl.h> to #include <gl3.h>. Maybe that helps? I tried pluggin in the power cord, and also disable the automatic graphis switching, but both did not help with this...

…o feat/add-graphics-scheduler

DarkWeird · 2022-03-24T07:29:22Z

@skaldarnar
Welcome to test :)
pollend update those shaders to 330.. but don't update splashscreen
don't forget MovingBlocks/SplashScreen#7 for resolve graphics glitches at splashscreen (at least several)

skaldarnar · 2022-03-24T08:32:39Z

The changes seem to help a bit, but it's still not working on (my) Mac 😕

The splash screen window appears before startup, but is rendered as flickering red triangle. No images or text are shown. I did check this with the MovingBlocks/SplashScreen#7 by doing the following, did not help - do I need to set up/configure anything else?

groovyw lib get SplashScreen
cd libs/SplashScreen
gh pr checkout 7

Eventually, the main menu shows up, but when I try to start/create a world the game crashes before anything is rendered on screen with the following error. The loading bar reaches the end and is "Catching World", it looks like it crashes right before rendering the first in-game frame...

FATAL ERROR in native method: Thread[GRAPHICS,5,main]: No context is current or a function that is not available in the current context was called. The JVM will abort execution.
        at org.lwjgl.opengl.EXTFramebufferObject.glBindFramebufferEXT(Native Method)
        at org.terasology.engine.rendering.dag.stateChanges.BindFbo.process(BindFbo.java:62)
        at org.terasology.engine.rendering.world.WorldRendererImpl$$Lambda$809/0x00000008007e0040.accept(Unknown Source)
        at java.util.ArrayList.forEach(java.base@11.0.11/ArrayList.java:1541)
        at org.terasology.engine.rendering.world.WorldRendererImpl.render(WorldRendererImpl.java:351)
        at org.terasology.engine.core.modes.StateIngame.render(StateIngame.java:223)
        at org.terasology.engine.core.subsystem.lwjgl.LwjglGraphics.lambda$postUpdate$1(LwjglGraphics.java:82)
        at org.terasology.engine.core.subsystem.lwjgl.LwjglGraphics$$Lambda$530/0x0000000800699040.run(Unknown Source)
        at reactor.core.publisher.MonoRunnable.subscribe(MonoRunnable.java:49)
        at reactor.core.publisher.MonoUsing.subscribe(MonoUsing.java:109)
        at reactor.core.publisher.Mono.subscribe(Mono.java:4399)
        at reactor.core.publisher.MonoSubscribeOn$SubscribeOnSubscriber.run(MonoSubscribeOn.java:126)
        at org.terasology.engine.core.schedulers.ThreadCaptureScheduler.start(ThreadCaptureScheduler.java:59)
        at org.terasology.engine.Terasology.main(Terasology.java:157)

…scheduler

DarkWeird added 2 commits August 24, 2021 11:34

feat(reactor): make separate graphic scheduler/thread

ec3f4ce

* splash screen's loop handles by Flux/Mono * Most calls to GL/GLFW runs at Graphic Thread via GameScheduler#runBlockingGraphics.

Merge remote-tracking branch 'origin/refactor/remove-usage-ThreadMana…

c11dc75

…gerSubsystem' into refactor/remove-usage-ThreadManagerSubsystem

DarkWeird requested review from keturn, pollend, Cervator, skaldarnar and jdrueckert August 24, 2021 08:42

github-actions bot added the Type: Improvement Request for or addition/enhancement of a feature label Aug 24, 2021

feat(reactor): add chooseable graphics scheduler (for TeraEd)

75686bb

* Some Docs

refactor(reactor): remove useless calls to asynchToDisplayThread

2ab9f06

* remove unused code

fix(reactor): fix using process actions by portlet

611d093

Base automatically changed from refactor/remove-usage-ThreadManagerSubsystem to develop August 27, 2021 23:43

keturn reviewed Aug 28, 2021

View reviewed changes

DarkWeird mentioned this pull request Aug 29, 2021

Refactor usage of concurrency with reactor #4798

Open

11 tasks

pollend and others added 5 commits September 4, 2021 11:37

chore: cleanup formatting

c11a6e8

Merge branch 'develop' of github.com:MovingBlocks/Terasology into fea…

94b81f2

…t/add-graphics-scheduler

Merge branch 'develop' into feat/add-graphics-scheduler

99b6ac6

fix(graphics-scheduler): fixes issue by PR

fb68f1b

Merge branch 'develop' into feat/add-graphics-scheduler

8345830

DarkWeird added 3 commits October 25, 2021 17:55

Merge branch 'develop' into feat/add-graphics-scheduler

a37a12c

Merge branch 'develop' into feat/add-graphics-scheduler

38f5fe7

# Conflicts: # engine/src/main/java/org/terasology/engine/rendering/primitives/ChunkMesh.java

Merge remote-tracking branch 'origin/develop' into feat/add-graphics-…

c201664

…scheduler # Conflicts: # engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglDisplayDevice.java # engine/src/main/java/org/terasology/engine/core/subsystem/lwjgl/LwjglGraphics.java

DarkWeird and others added 4 commits March 23, 2022 10:27

fix(graphic-scheduler): Update changes for graphic scheduler.

b1829d9

chore: remove unused imports

7a9cb56

chore: change variable declaration order

94641a3

Merge branch 'develop' into feat/add-graphics-scheduler

bb3ff81

DarkWeird added 2 commits March 24, 2022 10:24

fix(graphic-scheduler): drop splashscreen's shaders to version 1.2

ad65074

Merge remote-tracking branch 'origin/feat/add-graphics-scheduler' int…

f82dd10

…o feat/add-graphics-scheduler

DarkWeird mentioned this pull request Apr 5, 2022

Add Thread Naming Support #5005

Open

Merge remote-tracking branch 'origin/develop' into feat/add-graphics-…

8b53c7b

…scheduler

jdrueckert added this to the 5.4.0 milestone Sep 4, 2022

jdrueckert modified the milestones: Legacy - 5.4.0, 2023 Revive - Future Milestones Apr 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(reactor): make separate graphic scheduler/thread #4865

feat(reactor): make separate graphic scheduler/thread #4865

DarkWeird commented Aug 24, 2021 •

edited

DarkWeird commented Aug 24, 2021

skaldarnar commented Aug 24, 2021

DarkWeird commented Aug 24, 2021 •

edited

DarkWeird commented Aug 24, 2021

keturn left a comment •

edited

keturn Aug 27, 2021

DarkWeird Aug 28, 2021

keturn Aug 27, 2021

keturn Aug 28, 2021

DarkWeird Aug 28, 2021

keturn Aug 28, 2021

DarkWeird Aug 28, 2021

keturn Aug 28, 2021

keturn Aug 28, 2021

DarkWeird Aug 28, 2021

keturn Aug 28, 2021

keturn Aug 28, 2021

DarkWeird Aug 28, 2021

keturn Aug 28, 2021

DarkWeird Aug 28, 2021

keturn Aug 28, 2021

DarkWeird Aug 28, 2021

DarkWeird commented Sep 13, 2021

skaldarnar commented Mar 23, 2022 •

edited

DarkWeird commented Mar 24, 2022

skaldarnar commented Mar 24, 2022 •

edited

		return GameScheduler.runBlockingGraphics("hasFocus",
		() -> GLFW.GLFW_TRUE == GLFW.glfwGetWindowAttrib(GLFW.glfwGetCurrentContext(), GLFW.GLFW_FOCUSED));

		GameScheduler.runBlockingGraphics("isFullscreen", () ->
		glViewport(0, 0, width, height)

feat(reactor): make separate graphic scheduler/thread #4865

Are you sure you want to change the base?

feat(reactor): make separate graphic scheduler/thread #4865

Conversation

DarkWeird commented Aug 24, 2021 • edited

Contains

How to test

Outstanding before merging

After merge:

DarkWeird commented Aug 24, 2021

skaldarnar commented Aug 24, 2021

DarkWeird commented Aug 24, 2021 • edited

DarkWeird commented Aug 24, 2021

keturn left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarkWeird commented Sep 13, 2021

skaldarnar commented Mar 23, 2022 • edited

DarkWeird commented Mar 24, 2022

skaldarnar commented Mar 24, 2022 • edited

DarkWeird commented Aug 24, 2021 •

edited

DarkWeird commented Aug 24, 2021 •

edited

keturn left a comment •

edited

skaldarnar commented Mar 23, 2022 •

edited

skaldarnar commented Mar 24, 2022 •

edited