Skip to content

[BUG] Fixed passing of settings - #384

Open
najlkin wants to merge 1 commit into
masterfrom
fix-settings-pass
Open

najlkin wants to merge 1 commit into
masterfrom
fix-settings-pass

Conversation

@najlkin

@najlkin najlkin commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

This PR fixes passing of settings from the old data state to a new one. Among other things, it fixes the point line in a more systematic way than #378 . It also fixes passing of mesh attributes as mentioned in #383 .

@tzanio

tzanio commented Nov 1, 2026

Copy link
Copy Markdown
Member

🤖 Suggestions from an AI review. Feel free to ignore if not relevant.

  • A pointline command followed immediately by new solution data can lose the line: CloneEmpty() can copy the old coordinates before the queued POINT_LINE command executes. The wait in NewMeshAndSolution() happens after that copy. Could we preserve point coordinates on the main thread when applying the update, as in Retain point line when new data is sent #378, and avoid copying the live point-coordinate vector from the communication thread?

  • There may also be a data race: the worker’s CloneEmpty() copies point_coords while the main thread can replace that vector when executing POINT_LINE or installing new data. Could we avoid reading the live vector from the worker, or synchronize those accesses? Copying the coordinates again on the main thread would fix the stale-value issue but leave this concurrent access.

@najlkin

najlkin commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

🤖 is right, but both are corner cases. Setting a point line is fast, so it is unlikely code fetches old values in the next command. #378 does not suffer from that, but it is not systematic handle specially point line during update. The new data state should be allowed to be arbitrary - adding a line, removing, anything...

The data race is there too, as it always has been. Querying anything from the data state or visualization scene during stream processing is a potential data race condition. However, all the settings are just simple data types with near atomic access. The point line increases the chances, but GlvisCommand is designed to communicate stuff towards the main thread, not the other way around. Maybe I could add a mutex for the data state if you want... 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants