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 render issues, re-enable buffer compaction #6358

Merged
merged 3 commits into from
Apr 20, 2023

Conversation

kazcw
Copy link
Contributor

@kazcw kazcw commented Apr 19, 2023

Pull Request Description

  • Fix handling of a Symbol with no instances. We had logic to call context.draw_arrays (non-instanced) if instance_scope().size() == 0. This was incorrect: All Symbols are instanced symbols, and should only be drawn if at least one instance exists. This didn't cause a problem before because it was unreachable: We never instantiated a Symbol without creating at least one instance, and without buffer compaction it was not possible for instance count to decrease. Fixed by skipping all draw-related GL calls if there are no instances (this might also improve performance in some conditions).
  • Fix an incorrect OptVec iteration pattern (would cause a crash in conditions not currently reachable).
  • Re-enable buffer compaction.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@kazcw kazcw self-assigned this Apr 19, 2023
@kazcw kazcw linked an issue Apr 19, 2023 that may be closed by this pull request
@kazcw kazcw added the CI: No changelog needed Do not require a changelog entry for this PR. label Apr 19, 2023
@wdanilo wdanilo merged commit 824d18d into develop Apr 20, 2023
@wdanilo wdanilo deleted the wip/kw/fix-instancing-detection branch April 20, 2023 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix buffer compaction
2 participants