Skip to content

Siwin backend improvements#156

Open
xTrayambak wants to merge 2 commits into
elcritch:mainfrom
xTrayambak:siwin-backend-1
Open

Siwin backend improvements#156
xTrayambak wants to merge 2 commits into
elcritch:mainfrom
xTrayambak:siwin-backend-1

Conversation

@xTrayambak
Copy link
Copy Markdown

Hiya, I saw that you've implemented a Siwin backend. It doesn't compile yet though (as it's mostly a skeleton, as you mentioned).

This PR aims to add a proper backend that uses Siwin.

Please don't merge it right now - it's still very much a work in progress!

@elcritch
Copy link
Copy Markdown
Owner

elcritch commented Jun 5, 2025

Hiya, I saw that you've implemented a Siwin backend. It doesn't compile yet though (as it's mostly a skeleton, as you mentioned).

Awesome! Nah I've been wanting to add a Siwin backend but figured I'd wait to see what you got. I am planning to start working MacOS support for Siwin which apparently is broken. It'd be nice to default to Siwin in the future.

Also my work on adding the new rendering backend changes the windowing API a bit. It's the same methods, but the objects change a bit and will conflict with this PR. It should be pretty easy to fix though.

return ClipboardStr("")

method run*(renderer: RendererSiwin) =
renderer.window.run(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break things as the Figuro Renderer needs to control things and poll for events. It has it's own event system which controls when rendering needs to occur.

Windy/Windex had a poll and exposes a swapBuffer. It looks like Siwin doesn't expose a poll method, but has some swapBuffer methods.

I'm going to create an issue for Siwin about it.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually looks like it does have a way to do it! It'd just be using the firstStep and step methods. See: https://github.com/levovix0/siwin/blob/db2dfb2de848c2fb4f7e7950bb86ee229ebca9f5/src/siwin/platforms/any/window.nim#L402

Siwin's run proc is just:

proc run*(window: sink Window, makeVisible = true) =
  ## run whole window main loops
  window.firstStep(makeVisible)
  while window.opened:
    window.step()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably what would be needed on the Figuro side is to change the runRendererLoop or maybe the pollAndRender. Something like this:

proc runRendererLoop*(renderer: Renderer) =
  threadEffects:
    RenderThread

  siwinWindw.eventHandler.onRender = proc () =
    renderer.pollAndRender()

  siwinWindow.firstStep() # first step

  while app.running:
    siwinWindow.redraw() # request redraw
    siwinWindow.step()

    os.sleep(renderer.duration.inMilliseconds)

redraw

@elcritch
Copy link
Copy Markdown
Owner

elcritch commented Jun 5, 2025

@xTrayambak I'd recommend looking at my PR that refactors the render to be swappable. Also checkout my PR comments I left on the code. That'll save you a lot of headaches.

The swappable renderer in this PR might make it easier to integrate: https://github.com/elcritch/figuro/pull/155/files

The PR makes both a Renderer and a WindowRenderer objects:

RendererWindow* = ref object of RootObj

I can also modify that PR to make it easier to support Siwin as it's a bit different than Windy/Windex.

Maybe it'll have to be something fun like the window and the renderer having runRenderer methods so the windows library can wrap the renderer calls. Annoying and cumbersome but not too hard.

@xTrayambak
Copy link
Copy Markdown
Author

Yeah, that seems to make it a bit easier. I'll just wait until you merge that MR, then I'll continue work on this.

@elcritch
Copy link
Copy Markdown
Owner

elcritch commented Jun 8, 2025

Yeah, that seems to make it a bit easier. I'll just wait until you merge that MR, then I'll continue work on this.

@xTrayambak I just merged the PR refactoring the renderer.

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.

2 participants