Skip to content
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

[ui][fix] Edge: Fixing an issue with mouse event on Custom EdgeMouseArea causing Crash #2650

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

waaake
Copy link
Contributor

@waaake waaake commented Jan 20, 2025

Event::button() was sometimes causing a crash, copying the event and using event.buttons() from the copied event seems to be stable before calling event.button() eventually

Description

This PR addresses a known crash issue with the EdgeMouseArea (QQuickItem)'s mousePressEvent.
The crash occurs when querying button() from the event.
The event is not null and is able to provide other data correctly before ending up crashing when queried for button.

Features list

  • Fix UpdateShape to get invoked on geometryChange
  • Fix Alt + Mouse Click to Remove an Edge due to a change in data type for Keyboard Modifiers from int to enum.Flag
  • Fix Crash on clicking egde multiple times

Implementation remarks

Current Fix is based on copying the provided event and calling event.buttons() to check if the buttons are supported and then calling button() is the custom MouseEvent constructor.
This seems to have stopped the crashes for now, but still testing on larger scale is pending before we confirm anything.

@waaake waaake self-assigned this Jan 20, 2025
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.96%. Comparing base (6071a91) to head (870dcf4).
Report is 17 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2650      +/-   ##
===========================================
+ Coverage    69.93%   69.96%   +0.02%     
===========================================
  Files          121      121              
  Lines         7088     7084       -4     
===========================================
- Hits          4957     4956       -1     
+ Misses        2131     2128       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@waaake waaake force-pushed the fix/CrashingEdge branch 2 times, most recently from 6dc4ef0 to 5a116b4 Compare January 22, 2025 05:15
With Qt6 the geometryChanged method was updated to be called as geometryChange and with that update QQuickItem was not getting internal updateShape to get invoked upon updates
@waaake waaake marked this pull request as ready for review January 27, 2025 06:00
@fabiencastan fabiencastan added this to the Meshroom 2025.1.0 milestone Jan 27, 2025
@yann-lty yann-lty merged commit e51bb93 into develop Jan 27, 2025
5 checks passed
@yann-lty yann-lty deleted the fix/CrashingEdge branch January 27, 2025 11:22
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.

3 participants