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

Running multiple engines creates errors in image-renderer #2622

Closed
jalyna opened this issue Apr 30, 2023 · 2 comments · Fixed by #2635
Closed

Running multiple engines creates errors in image-renderer #2622

jalyna opened this issue Apr 30, 2023 · 2 comments · Fixed by #2635
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior

Comments

@jalyna
Copy link

jalyna commented Apr 30, 2023

First: Thank you for your amazing work around excalibur. I really enjoy working with excaliburjs ⚔️

After reading the contributing guidelines I hope this is a qualifies as a new issue. I've been working on a little 2d game and wanted to add a second engine since I want to display a mini map that has its own camera and actors.

I'm not able to run 2 engines simultaneously as the first created engine will not be drawn correctly or in a funny way.

I'm not sure if this is something that excalibur wants to support but since the graphicContext is so well isolated I was suprised I was running into this issue. I created a minimal example for this.

Steps to Reproduce

  1. Add 2 canvas elements to HTML
  2. Execute this code:
async function createEngines() {
 createEngine1();
  new Engine({
    canvasElementId: "canvas2",
    width: 200,
    height: 200,
    backgroundColor: Color.Yellow
  });
}

async function createEngine1() {
  const image1 = new ImageSource("/tiled/charsets/player1.png");
  const actor1 = new Actor({
    pos: vec(100, 100),
    width: 50,
    height: 50,
    color: Color.Red
  });
  const engine1 = new Engine({
    canvasElementId: "canvas1",
    width: 200,
    height: 200,
    backgroundColor: Color.Green
  });
  await image1.load();
  engine1.add(actor1);
  actor1.graphics.use(image1.toSprite());
  engine1.start();
}

 createEngines();

I also created a jsfiddle here: https://jsfiddle.net/kp1tLy4o/6/

As you can see I don't need to render anything for my second engine, it just has to exist.

This will not render the existing image and shows an error in the console:

Screenshot 2023-04-30 at 17 11 30

If I don't create the second engine everything looks like expected.

Expected Result

Screenshot 2023-04-30 at 17 12 27

I expect that the sprite is drawn.

Actual Result

Screenshot 2023-04-30 at 17 06 09

Sprite is not drawn and I see above error message.

Environment

  • browsers and versions: Chrome (110.0.5481.96), Safari (16.3)
  • operating system: MacOS 13.2.1
  • Excalibur versions: 0.27.0

Current Workaround

If I run the engine initialization sequentially (so adding an await createEngine1()) I don't see the error and the sprite is drawn. In my actual game though I will add new actors dynamically in later stages, so just drawing something once is not really the solution for me.

So far the issue only happens to me when using images, other resources have not been an issue.

@eonarheim eonarheim added the bug This issue describes undesirable, incorrect, or unexpected behavior label Apr 30, 2023
@eonarheim
Copy link
Member

Hi @jalyna

Thanks for the kind words! Much appreciated!

I agree multiple engines is definitely something that we'd want to be able to support.

I think I know where the issue is in Excalibur, the GL context is stored in a singleton so when a new engine is created it tries to use the GL context created from another canvas, which matches the error message ☹️ I have a couple ideas on how to fix this in Excalibur... I think some refactoring will be involved (removing the singleton entirely and passing gl context? using a service locator per engine instance? more complicated singleton to handle multiple contexts?)

Problematic code: https://github.com/excaliburjs/Excalibur/blob/main/src/engine/Graphics/Context/webgl-adapter.ts

Another possible temporary workaround until this is fixed would be to isolate each (or perhaps one) engine instance in it's own iframe? This might not be desirable understandably since iframes come with a lot of security baggage. You can use postMessage to communicate between the frames, perhaps this will work?

Here is a potential way to do it https://github.com/eonarheim/multi-excalibur-engine-workaround

Hope this helps until this is fixed!

@jalyna
Copy link
Author

jalyna commented Apr 30, 2023

@eonarheim Thank you so much for your quick answer 🙏🏻

I will try the iframe workaround, iframes these days are quite flexible so I think this will fit my use case. Thanks a lot!

eonarheim added a commit that referenced this issue May 6, 2023
Closes #2622 Multi engine support is theoretically here! 

There are some big changes here:
* TextureLoader is no longer a singleton, but an instance member on the `ExcaliburGraphicsContextWebGL`
* We defer loading textures into webgl until the first draw call, this is needed to in order to upload to the multiple contexts. This shouldn't cause too much of an issue since we only need to do this if the image ever changes.
* Additionally you must pass the gl context into the gl utility types

Needs a little more testing to make sure everything is still good. There is potentially some more refactoring opportunities as well to make this feel a little cleaner

<img width="1075" alt="image" src="https://user-images.githubusercontent.com/612071/236109108-0db4985e-7017-4609-8850-041fcd851806.png">


## Changes:

- Refactor TextureLoader to not be static 
- Remove webgl adapter and pass gl context to gl helper types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants