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

Fix race condition when rendering the UI #774

Merged

Commits on May 9, 2021

  1. Fix race condition when rendering the UI

    This fix depends on a fix in ign-rendering module, because it
    depends on the new Camera::SwapFromThread function
    Without it, compilation will fail
    
    Affects gazebosim/gz-rendering#304
    
    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 9, 2021
    Configuration menu
    Copy the full SHA
    38e0a59 View commit details
    Browse the repository at this point in the history
  2. Force serialization of render commands

    This avoids us to break ign-rendering ABI while also simplifying the
    amount of work to be done
    
    Serializing work is easier to maintain and debug
    Only CPU-bound scenario would potentially benefit from parallel command
    generation (in terms of UI responsiveness)
    Parallel command generation can be added back later
    
    Also fixed coding style
    
    Refer to
    gazebosim/gz-rendering#304 (comment)
    for discussion
    
    Affects ign-rendering#304
    
    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 9, 2021
    Configuration menu
    Copy the full SHA
    29be301 View commit details
    Browse the repository at this point in the history
  3. Fix deadlock on initialization & shutdown

    Also fixes preexisting race condition when shutting down and improper
    uninitialization of the worker thread
    
    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 9, 2021
    Configuration menu
    Copy the full SHA
    8100758 View commit details
    Browse the repository at this point in the history
  4. Fix deadlock when using MoveTo modifier

    Fix coding style
    
    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 9, 2021
    Configuration menu
    Copy the full SHA
    081e7d0 View commit details
    Browse the repository at this point in the history
  5. Reimplemented synchronization mechanism to fix deadlocks

    It's a good thing we went for serializing rendering.
    
    THe way Qt implements the double buffer scheme using signals & slots is
    fundamentally flawed because it assumes the worker thread never needs to
    synchronize (e.g. to invalidate FBOs if window resolution changes).
    
    Trying to synchronize can easily cause deadlocks if Qt thread has
    spurious updates which don't end up emitting TextureInUse, as the worker
    thread is running slower than Qt thread.
    
    A way to fix this could be to use a different synchronization mechanism
    where the main thread increases a request counter and the worker thread
    is constantly looping but only wakes up when that counter is > 0.
    
    For now, this will do.
    
    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 9, 2021
    Configuration menu
    Copy the full SHA
    c236828 View commit details
    Browse the repository at this point in the history

Commits on May 10, 2021

  1. Document RenderSync

    Turn some pointers into references
    
    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 10, 2021
    Configuration menu
    Copy the full SHA
    e1583ba View commit details
    Browse the repository at this point in the history

Commits on May 15, 2021

  1. Document missing parameter

    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 15, 2021
    Configuration menu
    Copy the full SHA
    d55468d View commit details
    Browse the repository at this point in the history

Commits on May 23, 2021

  1. Document better our use of emit TextureInUse

    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 23, 2021
    Configuration menu
    Copy the full SHA
    55b2a7e View commit details
    Browse the repository at this point in the history
  2. Change Q_SLOTS to slots for consistency

    I changed it because it didn't work, but it must've been old code
    because it works now.
    
    Signed-off-by: Matias N. Goldberg <[email protected]>
    darksylinc committed May 23, 2021
    Configuration menu
    Copy the full SHA
    6285e23 View commit details
    Browse the repository at this point in the history

Commits on May 24, 2021

  1. Configuration menu
    Copy the full SHA
    ec154fe View commit details
    Browse the repository at this point in the history