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

Minor driver cleanup #4403

Merged
merged 4 commits into from
Jan 7, 2024
Merged

Minor driver cleanup #4403

merged 4 commits into from
Jan 7, 2024

Conversation

Jacalz
Copy link
Member

@Jacalz Jacalz commented Nov 18, 2023

Description:

It was a failure to reapply the init changes. Too many problems surface. I kept the other improvements from the PR and cleaned up some more things.

  • The animation runner struct object didn't need to be a pointer. This allows us to not manually create the object, which in turn means that we get rid of some code and the constructor for the GLDriver is thus now inlineable.
  • A macOS-specific function got updated to avoid multiple interface casts.
  • The glDevice struct does not need to part of the driver. It is an empty struct, we can just return it each time someone asks for it.

This replaces my previous PR which had to be reverted due to an issue with the application not having started. This cleans up some of the init code to avoid calling init multiple times (when starting and when creating a new windows). It also includes an extra change to set up the device field more coherently.

Replaces #4177

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

@coveralls
Copy link

coveralls commented Nov 18, 2023

Coverage Status

coverage: 64.548% (+0.001%) from 64.547%
when pulling 2968c87 on Jacalz:reapply-init-pr
into b49d90c on fyne-io:develop.

@andydotxyz
Copy link
Member

Hmm, not sure.
This moves the OpenGL init from "when showing first window" to "when the app is loaded".
So it's much earlier in the lifecycle. Is this a good thing?
If an app loads and never shows a window we don't need OpenGL to be loaded before but now we do...

@Jacalz
Copy link
Member Author

Jacalz commented Dec 17, 2023

Hmm. I think you're right but I'm really not a fan of the way it is called so many times unnecessarily now. Maybe there is a better way. I need to think about it...

@Jacalz Jacalz marked this pull request as draft December 25, 2023 11:34
@Jacalz Jacalz changed the title Cleanup GLFW init to avoid needing a sync.Once Minor driver cleanup Jan 6, 2024
@Jacalz
Copy link
Member Author

Jacalz commented Jan 6, 2024

I haven't managed to come up with any good solutions. I rebased the PR and kept one of the other improvements from it plus expanded a bit on that idea. Now it's just a small cleanup.

@Jacalz Jacalz marked this pull request as ready for review January 6, 2024 22:26
@Jacalz Jacalz merged commit 4e5e404 into fyne-io:develop Jan 7, 2024
12 checks passed
@Jacalz Jacalz deleted the reapply-init-pr branch January 7, 2024 17:37
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.

3 participants