-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
improve docs for GLMakie backend activation #2653
Conversation
mostly wordsmithing, a format fix, and a description of the `visible` option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I had a couple of minor comments, but it looks great, and I love when someone pays attention to the docs :)
Since #2544 is making some changes in this region as well, I'd like to wait to merge this until that is merged, so we don't face any annoying merge conflicts.
* `monitor::Union{Nothing, GLFW.Monitor} = nothing`: Sets the monitor on which the Window should be opened. | ||
* `fullscreen = false`: Whether to start the window in fullscreen mode. | ||
* `debugging = false`: If `true`, starts the GLFW.Window/OpenGL context with debug output. | ||
* `monitor::Union{Nothing, GLFW.Monitor} = nothing`: Sets the monitor on which the window should be opened. If set to `nothing`, GLFW will decide which monitor to use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to add how you actually set the monitor here...I think there's some discussion of this in #2642.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll admit I haven't tried out most of this functionality myself. From #2642 it looks like setting the monitor doesn't currently work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it looks like the current setting method doesn't do what you'd think it does (set the monitor on which the window should be displayed).
Since GLFW uses X directly, I guess the best way to do this would be to set ENV["DISPLAY"]
before launching the window? CC @SimonDanisch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I don't think that's a good idea. I'm not 100% sure if that even works, and even if it does on linux, it won't work on windows.
So, I think we should just fix it to allow passing a GLFW.Monitor
Co-authored-by: Anshul Singhvi <[email protected]>
Thank you! :) |
Description
This is mostly wordsmithing in docs, docstrings, and comments. Also includes a format fix for an admonition and a description of the
visible
option that was added to ScreenConfig recently (#2587).Type of change
This only changes docs and fixes typos in a couple of comments.
Checklist