microscope-cockpit / cockpit

Cockpit is a microscope graphical user interface. It is a flexible and easy to extend platform aimed at life scientists using bespoke microscopes.
https://microscope-cockpit.org
GNU General Public License v3.0
37 stars 27 forks source link

Mac macrostage data display for position and steps size are too small. #843

Closed iandobbie closed 1 year ago

iandobbie commented 1 year ago

The mac macrostage window looks like the image below, chopping of the actaul stage position and step size.

Screen Shot 2023-01-26 at 4 25 55 PM
iandobbie commented 1 year ago

I hacked this by making the text boxes a fixed width but this isn't a good solution as it bypasses the sizer layout engine. How do we make the sizer give them the right size to start with and why are the saved top and saved bottom positions wider?

thomasmfish commented 1 year ago

Looking at the code (I'm not able to test this), I have a few suggestions that may help:

iandobbie commented 1 year ago

None of this worked because the text boxes where empty when the size was called. As the text format is fixed as 5.2f and 4.2f for the position and step size respectively I prefilled them with 00000.00 and 0000.00 at creation and it all just worked. Fix is in https://github.com/iandobbie/cockpit/tree/macrostage-text-fix. The actual values are pulled before you notice anything, so I think this is a pretty good fix. I will test on a PC and then push into the main branch.

I will test it on the PC but I'm pretty sure this will just work

thomasmfish commented 1 year ago

I think there's a slightly nicer way of doing this, if you specify the text control size based on the following line: self.GetSizeFromText("88888.88")

(using 8s as that's the widest digit)

If GetSizeFromText isn't available (new in wx version 4.1), it's only a convenience function for this: self.GetSizeFromTextSize(self.GetTextExtent(text).GetWidth())

All of this where self is an instance of wx.Control

iandobbie commented 1 year ago

Good suggestion, Not quite got it to work yet, I'll have a fiddle and see if I can make it work.

iandobbie commented 1 year ago

What I settled on is self.SetInitialSize(self.GetSizeFromText("88888.88")) in the position control and with one less 8 in the step size control. Seems to work on the mac, will test on PC and push to main branch

thomasmfish commented 1 year ago

Can you not set it like this? super().__init__(parent,size=self.GetSizeFromText("88888.88"),style=wx.TE_RIGHT|wx.TE_READONLY) The init for wx.TextCtrl is __init__ (self, parent, id=ID_ANY, value=””, pos=DefaultPosition, size=DefaultSize, style=0, validator=DefaultValidator, name=TextCtrlNameStr), so size should be settable there

EDIT: should just link the doc https://docs.wxpython.org/wx.TextCtrl.html?highlight=textctrl#wx.TextCtrl

iandobbie commented 1 year ago

I did try that first but it didn't seem to work. I'll try again.

iandobbie commented 1 year ago

Nope definitely doesnt work for me....

File "/Users/ID/src/cockpit/cockpit/gui/macroStage/macroStageWindow.py", line 67, in init super().init(parent,size=self.GetSizeFromText("88888.88"),style=wx.TE_RIGHT|wx.TE_READONLY) RuntimeError: super-class init() of type HandlerPositionCtrl was never called

But no other obvious error messages.

thomasmfish commented 1 year ago

Oh, I think GetSizeFromText probably depends on the style, so can't be called until after __init__ - sorry about that! In that case would self.SetMinSize be best? I'm not sure what initial sets but I'm guessing a minimum size is the most important size?

iandobbie commented 1 year ago

Not sure but boxes obviously aren't growing to the size of the text on macos, so I think setting initial size is ok, I'll what set min size does.

iandobbie commented 1 year ago

So SetMinSize produces exactly the same result as the SetInitialSize so I guess it is preferential as of something later wants to change the size there will still be a sensible minimum.

thomasmfish commented 1 year ago

Yeah, my guess is you could end up with squished boxes, depending on some flags and resizing, with SetInitialSize but they probably both practically do the same if wx.EXTEND is not set as a flag.

iandobbie commented 1 year ago

Ok, I just checked it on windows and it works fine there so I'll push it. Closing this discussion