Skip to content

Fixes Ogl when resizing and dragging shapes #2743

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hasii2011
Copy link

@hasii2011 hasii2011 commented May 2, 2025

[
Many of the computations when resizing or moving OGL shapes result in float values. The result is that the wx.DC primitive calls fail because they expect integer values. This PR changes the computations results to become integer values, thus method calls on the wx device contexts do not fail

Most of the changes involve

  • using the integer divide operator '//' instead of the float operator '/'
  • In some instances I massage the results via the 'round' operator
  • Additonally, I change the use of wx.RealPoint to just wx.Point
  • Finally, I changed some of the float constants to integer constants
    ]

Fixes #2739

@hasii2011 hasii2011 marked this pull request as draft May 2, 2025 15:59
[
* Attachment points are computed as float
]

[wxWidgets#2739]
@hasii2011 hasii2011 marked this pull request as ready for review May 2, 2025 22:29
@hasii2011 hasii2011 marked this pull request as draft May 3, 2025 03:55
[
* Don't use RealPoint
* Do integer arithmetic
]

[(s)]
@hasii2011 hasii2011 marked this pull request as ready for review May 3, 2025 15:00
@hasii2011
Copy link
Author

hasii2011 commented May 3, 2025

Want to get these set of changes into a daily build. If I need more patches I can work against it. I verified these changes by modifying the virtual environment I created.

@echoix
Copy link
Contributor

echoix commented May 3, 2025

Can you either edit the title or explain what that markup in the commit message and the PR title means?

@hasii2011 hasii2011 changed the title <>[]: <Fixes Ogl when resizing and dragging> Fixes Ogl when resizing and dragging shapes May 3, 2025
@hasii2011
Copy link
Author

I hope the above is clearer. Please advise

@echoix
Copy link
Contributor

echoix commented May 3, 2025

Couple things for the reviewers to watch for:

  • The change from wx.RealPoint to wx.Point seems a bit naive for the type of fix here, it changes a big part of the meaning.
  • Same thing with the overall changes in the file. Is it expected to do all intermediate calculations as integers, not just at the output? How does wxPython handle fractional scaling? The handling seems different between platforms for wxWidgets at least: https://docs.wxwidgets.org/3.2/overview_high_dpi.html
  • Not fixing the line 3447 as mentioned in the thread
  • Replacing a float() call with round(). Appart from changing the meaning greatly, the use of round() in the context of a graphics API surprises me. Graphics APIs usually hate off-by-one-pixel bugs, so a closer look needs to be taken to make sure the mathematical premises (assumptions) are still true after the change.

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.

wx.lib.ogl Float issues
2 participants