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

Update internal state before message send on select (was click-drag-borked) #1270

Closed
Andreya-Autumn opened this issue Sep 6, 2024 · 8 comments · Fixed by #1358
Closed

Update internal state before message send on select (was click-drag-borked) #1270

Andreya-Autumn opened this issue Sep 6, 2024 · 8 comments · Fixed by #1358
Labels
Bug Report Item submitted using the Bug Report template UI Issues related to UI look&feel

Comments

@Andreya-Autumn
Copy link
Collaborator

Make some empty zones. Click them one by one. Strange things happen

What

@Andreya-Autumn Andreya-Autumn added Bug Report Item submitted using the Bug Report template UI Issues related to UI look&feel labels Sep 6, 2024
@baconpaul
Copy link
Contributor

whoa that's bad. i wonder when that started. i'll look at this one as a priority.

@baconpaul
Copy link
Contributor

The log message shows

src/engine/group.cpp:499 [20:52:18.424] Implement Group LFO modulator use optimization (Message will only appear once)
src/selection/selection_manager.cpp:357 [20:52:24.666] guaranteeSelectedLead Be careful - we are promoting leadZone[selectedPart]=zoneaddr[p=0,g=0,z=1] 
src/selection/selection_manager.cpp:357 [20:52:25.420] guaranteeSelectedLead Be careful - we are promoting leadZone[selectedPart]=zoneaddr[p=0,g=0,z=1] 

If I had to guess I would look hard at 0babc62

Note that it only started happening for me when i added a second group.

@baconpaul
Copy link
Contributor

OK smallest possible repro is this

empty SC
create 2 zones - seems OK
adds a 3 zone - seems OK most of the time
add a group and click around - blows out almost immediately

so need to run this with the selection debugger to see if its a back end or up problem.

Will work on this as priority next.

@baconpaul
Copy link
Contributor

baconpaul commented Sep 6, 2024

Ahh this is an evil bug

what seems to be happening is the selection action is pushing the prior selected zone onto the new selected zones bounds

Screenshot 2024-09-05 at 9 17 09 PM

start here and click around some and the "zone disappears"
Screenshot 2024-09-05 at 9 17 24 PM

but it hasn't disappeared. What has happened is zone 2 and 3 are now co-incident.

so it's not the response its the outbound selection message getting screwed up somewhere.

Cool knowing that i should be able to find it tomorrow.

@baconpaul
Copy link
Contributor

Oh ha no you know what it is

its the change to allow you to move when you click

we are generating a very small move when we select and its racing to the back end with the wrong selected zone

I can fix this tomorrow but it's just a micro-drag when you click moving something by zero at the same time you select it and sending the wrong zone as being updated to the server.

@baconpaul
Copy link
Contributor

Yeah

void MappingDisplay::mappingChangedFromGUI()
{
    SCLOG("Mapping Changed from GUI");
    sendToSerialization(cmsg::UpdateLeadZoneMapping(mappingView));
}

that fires on a 'click to disappear'

will find this now i know that. but not tonight.

@Andreya-Autumn
Copy link
Collaborator Author

Nice hunting!

baconpaul added a commit to baconpaul/shortcircuit-xt that referenced this issue Sep 6, 2024
In  bd2b2e4 we made select start a drag event but this meant
tiny mosue movements when selecting would send a drag event simultaneously
with the select event on the wrong zone, making zones overlap.

The correct fix for this is to wait for the new selected zone or update
the view internally when selecting, so this is not a good solution
long term, buit this fixes the bug in the interim.

Addresses surge-synthesizer#1270
@baconpaul
Copy link
Contributor

Yeah I have a temporary workaround (which is you have to drag at least 2 pixels from mouse down spot to start a move) which is going in now to make this less annoying immediately but I also know what the correct fix is (namely when you do a select you update the internal state before you send the message, basically - but that's trickier).

baconpaul added a commit that referenced this issue Sep 6, 2024
In  bd2b2e4 we made select start a drag event but this meant
tiny mosue movements when selecting would send a drag event simultaneously
with the select event on the wrong zone, making zones overlap.

The correct fix for this is to wait for the new selected zone or update
the view internally when selecting, so this is not a good solution
long term, buit this fixes the bug in the interim.

Addresses #1270
@baconpaul baconpaul changed the title Click to select zone is very borked Update internal state before message send on select (was click-drag-borked) Sep 6, 2024
baconpaul added a commit to baconpaul/shortcircuit-xt that referenced this issue Sep 20, 2024
Fixes surge-synthesizer#1270 properly. Makes sure the internal state for selected
lead zone is set *before* we message the server to avoid inconsistent
messages. Kepp the drag window though just to be safe.

Closes surge-synthesizer#1270
baconpaul added a commit that referenced this issue Sep 20, 2024
Fixes #1270 properly. Makes sure the internal state for selected
lead zone is set *before* we message the server to avoid inconsistent
messages. Kepp the drag window though just to be safe.

Closes #1270
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report Item submitted using the Bug Report template UI Issues related to UI look&feel
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants