fix: Override Wx.RenderCanvas.set_logical_size #159
Conversation
Calling SetSize on a wx.Frame takes the title bar, window borders, etc. into account. As a result, the renderable area is slightly smaller (at least on Windows) than the dimensions provided to set_logical_size. This commit rectifies the behavior such that if set_logical_size is called on wx.RenderCanvas, wx.Frame.SetClientSize is called instead, which sets the internal size (i.e. not including title bar, window borders, etc.) instead. It also adds a test ensuring the behavior is now corrected (there isn't a ton of precedence for tests, these are modeled off test_gui_glfw)
This is actually a temporary fix, as it copies changes also made upstream to rendercanvas. We can remove these changes once we can depend upon pygfx/rendercanvas#159
| if self.Parent is not None: | ||
| width, height = int(width), int(height) | ||
| self.SetSize(width, height) | ||
| else: |
There was a problem hiding this comment.
I think that it'd be sufficient to just change the original's:
- parent.SetSize(width, height)
+ parent.SetClientSize(width, height)Then the changes to the WxRenderCanvas should not be necessary. I updated the test to also test for the initial size.
There is indeed a bit of a back-and-forth going on, where wrapper.set_logocal_size() calls the subwidget.set_logical_size() which then sets the size of the wrapper again. I think you were trying to avoid that, but the problem is that sometimes (most notably during initialization) the size is set from the subwidget.
There was a problem hiding this comment.
Could you try that on your machine? It works for me on Mac.
There was a problem hiding this comment.
Could you try that on your machine? It works for me on Mac.
Yes, also works for me on Windows.
There is indeed a bit of a back-and-forth going on, where
wrapper.set_logocal_size()calls thesubwidget.set_logical_size()which then sets the size of the wrapper again. I think you were trying to avoid that, but the problem is that sometimes (most notably during initialization) the size is set from the subwidget.
Yeah, this was my original solution, but as you noted I didn't really like setting the size of the parent.
But, as you mentioned, I think the ability of setting the size from the subwidget currently would not work with the current state of things. I was just looking a little bit into the logic using BaseRenderCanvas.__kwargs_for_later and BaseRenderCanvas._final_canvas_init, but maybe that's too involved for now 😅
Let's just do that one-line change you have! 👍
|
I went ahead and applied my suggestions. I'll do a minor release quickly to also get #160 released asap. We can re-iterate if needed. |
2 participants
Calling SetSize on a wx.Frame takes the title bar, window borders, etc. into account. As a result, the renderable area is slightly smaller (at least on Windows) than the dimensions provided to set_logical_size.
This PR rectifies the behavior such that if set_logical_size is called on wx.RenderCanvas, wx.Frame.SetClientSize is called instead, which sets the internal size (i.e. not including title bar, window borders, etc.) instead.
This PR also adds a test ensuring the behavior is now corrected (there isn't a ton of precedence for tests, the current tests are modeled off
test_gui_glfw.py)