avidemux2 icon indicating copy to clipboard operation
avidemux2 copied to clipboard

[crop/qt5] spinbox bugfix

Open szlldm opened this issue 4 years ago • 5 comments

-on change set spinbox maximums, to prevent inversion (bad UX) -these maximums prevent to set output resolution lower than 8x8 -spinboxes get minimum width, to prevent jerkiness (introduced by setMaximum)

szlldm avatar Jan 19 '21 14:01 szlldm

I'm sorry for not responding here earlier.

After my struggle with mplayerDelogo which led to 1b7ebcebfeb24cebad8b002c881654a74eff8946, I fear that Qt doesn't allow setting maximum for a spinbox dynamically while keeping up/down buttons auto-repeat working. Does auto-repeat still work for you in all spinboxes with this change?

I think, minimum width and height should be enforced at parameter validation and other relevant locations in code, also beyond this filter. I just lack a vision how to design this properly at the moment. Last but not least, if a filter may reject input, it must implement a pass-through operation.

eumagga0x2a avatar Feb 08 '21 09:02 eumagga0x2a

Does auto-repeat still work for you in all spinboxes

Well, it doesn't work here either. I have only tested for up/down arrow keys, those works.

szlldm avatar Feb 08 '21 11:02 szlldm

After my struggle with mplayerDelogo which led to 1b7ebce, I fear that Qt doesn't allow setting maximum for a spinbox dynamically while keeping up/down buttons auto-repeat working. Does auto-repeat still work for you in all spinboxes with this change?

I did a proof-of-concept on this, using PySide6. It seems that auto-repeat is only broken if you update the maximum of the current spinbox, while it's being changed. As long as you're careful to only update other spinboxes' maxima, but leave the current widget alone, auto-repeat works fine.

And that should be sufficient, because changing any spinbox's value can't affect its own maximum value. That should remain static, whatever the current value is set to. Whenever the code was updating maximum values, it would've always been calling setMaximum() on the current widget with the same value it had before. So, the solution is to add logic to... like... not do that. :rofl: If the top value is updating, only call bottom.setMaximum(), as that's the only maximum that could possibly change based on the value of top. If left is changing, only call right.setMaximum(). And so on.

ferdnyc avatar Jan 03 '23 02:01 ferdnyc

My proof-of-concept, if you want to try it yourself, is in ferdnyc/spinbox_test. Just clone it and run python3 spinbox_test.py from any Python installation/environment that has PySide6 available, and you'll get this dialog:

image

The "Box Totals:" values will be updated based on the spinbox values and maximum values. Auto-repeat on the spinbox arrows works unless "Update all spinboxes" is checked.

ferdnyc avatar Jan 03 '23 02:01 ferdnyc

Thanks, will look into that.

szlldm avatar Jan 04 '23 02:01 szlldm