search

ioquake / ioq3

The ioquake3 community effort to continue supporting/developing id's Quake III Arena
https://ioquake3.org/
GNU General Public License v2.0
2.38k stars 528 forks source link

Emscripten build improvements #658

Closed jdarpinian closed 3 months ago

jdarpinian commented 5 months ago

EDIT: This PR used to include GL4ES but I've removed it now that GLES is supported directly. Check the latest comments for updates.

This adds a web build using Emscripten to build a wasm binary and GL4ES to translate OpenGL 1 to WebGL. The vast majority of this code is GL4ES, imported as a dependency. To review just the ioquake3 changes, which are minor besides the Makefile, check the "Web build support with Emscripten" commit 7618f100ad2e88818e0ebaeb9f5ded818680fd72.

jdarpinian commented 4 months ago

I made a GitHub Actions workflow for the web build. You can download the generated artifacts and try out the web build yourself! https://github.com/jdarpinian/ioq3/actions/runs/8964848533/artifacts/1475480879

To run the web build you'll need to start a local webserver that serves both your baseq3 directory and the ioquake.html file. Then load ioquake.html?pakPathname=/path/to/your/baseq3 in your browser and play!

jdarpinian commented 4 months ago

I fixed a GL4ES issue that was causing the text in the console to be invisible as well as the mouse pointer in the main menu. Those were the only rendering issues I was aware of. As far as I know all rendering should be working 100%. I think this should be mergeable.

briancullinan2 commented 3 months ago

I had trouble with emscripten implementation of SDL_audio and sound spatializing, does it not use 30%+ CPU in this build?

I build Quake3e with WASI SDK only and it's much cleaner and faster than emscripten.

In my build the biggest pitfall is I started to convert the SND_Respatialize function to JavaScript Audio API.

NuclearMonster commented 3 months ago

Thanks James, this looks great in general, I have created #663 to look to the future and solve the underlying issue that causes ioq3 to require gl4es at this time, but a great solution now is better than the wishes and dreams of feature requests.

NuclearMonster commented 3 months ago

Can't figure out why Github won't let me re-run the actions, will try closing and reopening the PR

kungfooman commented 3 months ago

Is this a clean copy of @ptitSeb's gl4es?

Very nice work!

@zturtleman Didn't you do a lot of changes that make rend2 nearly GLES2? I ported it to WebGL some years ago and I don't know what changed in the meantime, but my starting point was rend2 and it more or less worked with some glitches tho out of the box (IIRC it was picky about defining the correct shader versions and stuff like that mostly).

zturtleman commented 3 months ago

@kungfooman yeah. I have a working OpenGL ES 2 port of the opengl2 renderer. #664

jdarpinian commented 3 months ago

@briancullinan2 Does your WASI build run in the browser easily? I haven't profiled this build to see what's using CPU.

@NuclearMonster Thanks! I have noticed one issue that probably should be investigated before merging. I mentioned that I fixed a GL4ES issue that was causing console text to be invisible, in commit 77017151858cb47e7f902ea655df2247e03498df. Unfortunately the fix was to disable a big optimization that GL4ES pretty much relies on for performance. Although rendering is correct, it is slow on anything but very high-end hardware. So I think the underlying issue with that optimization ought to be fixed before merging this. Or, I see you just merged ES2 support so maybe we could just use that and drop GL4ES? It might be nice to have GL4ES anyway so both the GL1 and ES2 backends could be used.

@kungfooman Thanks! Yes, it's a clean copy of GL4ES, just with some nonessential files removed. The commit ID is in code/gl4es/README.ioq3.md

NuclearMonster commented 3 months ago

Or, I see you just merged ES2 support so maybe we could just use that and drop GL4ES? It might be nice to have GL4ES anyway so both the GL1 and ES2 backends could be used.

With the unexpected ES2 PR thanks to Zack Middleton, I'd prefer to drop GL4ES for fewer dependencies and focus improvements on performance of the "OpenGL2/ES2" renderer if that looks possible. Sorry to disrupt your flow here. I really appreciate the work you put into bringing GL4ES over. Didn't expect to have wild dreams of dedicated ES2 support realized in a day!

jdarpinian commented 3 months ago

Makes sense to me, GL4ES is a big dependency. I'll drop GL4ES from the PR when I have some time to spend on it.

ptitSeb commented 3 months ago

I'm curious to know if you see any performances changes when switching from gl4es to gles2.

kungfooman commented 3 months ago

I'm curious to know if you see any performances changes when switching from gl4es to gles2.

Yea, me too, I just tested rend1 and rend2 on a Raspberry Pi 4 yesterday and rend1 got around 60-80 FPS and rend2 only around 10 FPS... no clue what the bottle neck is, but now I also want to test:

I don't wanna be too crazy, but I'm also thinking about libangle GLES to Vulkan... that may help performance as well.

zturtleman commented 3 months ago

I added basic emscripten support with the OpenGL2 renderer in https://github.com/ioquake/ioq3/pull/666 (https://github.com/ioquake/ioq3/commit/f41bd37fde1ef7aee59522ad6a5124a0eb6b00ba). This isn't meant to deter other changes.

briancullinan2 commented 3 months ago

@briancullinan2 Does your WASI build run in the browser easily? I haven't profiled this build to see what's using CPU.

https://quake.games

briancullinan2 commented 3 months ago

I'm curious to know if you see any performances changes when switching from gl4es to gles2.

Yea, me too, I just tested rend1 and rend2 on a Raspberry Pi 4 yesterday and rend1 got around 60-80 FPS and rend2 only around 10 FPS... no clue what the bottle neck is, but now I also want to test:

  • Zack's new ioq3 OpenGL ES
  • rend1 + your gl4es

I don't wanna be too crazy, but I'm also thinking about libangle GLES to Vulkan... that may help performance as well.

I think webgpu is the way forward? That would allow for vulkan support?

briancullinan2 commented 3 months ago

Just a warning for future development, I was able to trace a lot of performance issues back to emscripten itself.

https://github.com/inolen/quakejs/issues/44

Ghost is my comments from an account I closed. My plan to fix the sound issue was to implement 16/32 boom boxes and move them around for each channel.

https://developer.mozilla.org/en-US/docs/Web/API/Web_Audio_API/Web_audio_spatialization_basics

NuclearMonster commented 3 months ago

In order to keep this PR on-topic, please continue any general discussions about the web version of ioquake3 that aren't specific to this PR to our forum: https://discourse.ioquake.org/t/ioquake3-web-version-improvements-discussion/1961

jdarpinian commented 3 months ago

@zturtleman I just tried your ES renderer and it is faster than GL4ES, perhaps unsurprisingly. Very cool! I don't think we need to keep GL4ES as an option.

CPU time is low, but I do still see random long GPU frames. I'm not sure what's causing that.

kungfooman commented 3 months ago

CPU time is low, but I do still see random long GPU frames. I'm not sure what's causing that.

You mean in this PR or Zacks? Good thing about e.g. Chrome browser is that you can make a very detailed performance trace analysis: https://developer.chrome.com/docs/devtools/performance

jdarpinian commented 3 months ago

OK I've removed GL4ES from this PR. Still some changes remaining:

zturtleman commented 3 months ago

Okay so, personally I would rather merge this manually in pieces than request a lot of changes to the PR. Do you have a preference? (I also need to do some testing to see if --preload-file code path is worth keeping.)

I would also like to merge the STACK_SIZE fix first so the present build works with present emscripten.

zturtleman commented 3 months ago

I guess mainly I would like the STACK_SIZE fix as the first commit and for the commits undoing changes (like gl4es) or minor fixes to your other commits (like Fix web workflow) to be combined into the original commit. It would be nice if the commits were separate commits for logically separate high-level features similar to your list of changes. Other things can be changed later.

jdarpinian commented 3 months ago

OK I rebased this change on your PR #669. I also removed noise commits and separated features into different commits. You can merge #669 first if you want.

zturtleman commented 3 months ago

It looks good, thank you.