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

Do not use global init functions #606

Merged
merged 3 commits into from
Sep 5, 2019
Merged

Conversation

zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Sep 4, 2019

This allows walk to be used inside other applications without the overhead of the UI parts aren't used.

@zx2c4 zx2c4 mentioned this pull request Sep 4, 2019
@JoshuaSjoding
Copy link
Contributor

This accomplishes the primary goal of #601. It moves even more things out of init() which I think is great.

@zx2c4
Copy link
Contributor Author

zx2c4 commented Sep 4, 2019

Indeed it's a good start. Not totally there, as there still are globals, and you don't want globals at all. But it, at the very least, is a step in the right direction.

@JoshuaSjoding
Copy link
Contributor

@zx2c4 Do you plan to pursue removal of the globals? If not then I can take a stab at it with a separate pull request, but I don't want to conflict with any of your current efforts.

@lxn lxn merged commit 015262c into lxn:master Sep 5, 2019
@zx2c4
Copy link
Contributor Author

zx2c4 commented Sep 5, 2019

@JoshuaSjoding Maybe, but I wonder what the value would be. Not all globals need to be per-thread. I would be interested in doing something smarter with the ToolTip situation, as at the moment we're using the non-blocking cmpxchg instead of sync.Once, because the tooltip init functions calls InitWindow resulting in a deadlock. So getting that settled might be a more immediate priority. scjalliance@adb0efb might be a way of fixing it, but there's also a performance hit to doing that too (the tree traversal), so I'm not quite sure yet.

@zx2c4 zx2c4 deleted the jd/noglobalinit branch September 5, 2019 20:05
@JoshuaSjoding
Copy link
Contributor

@zx2c4 I think I have a better design in mind now. I'm going to create a WindowGroup type that stores state for a group of windows that share a thread, then store the common ToolTip control in the WindowGroup. I'll prepare a PR if I'm happy enough with the result.

@JoshuaSjoding
Copy link
Contributor

@zx2c4 I've prepared #610 as an alternative solution to #601.

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.

5 participants