Fix stack-overflow when creating a new atlas #949
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When an atlas is full and the `insert` call fails, a new atlas should be created. This is the current strategy, however the atlas is put at the end of the vector, but the `current_atlas` index is set to 0, instead of the last element. This leads to a recursion where it keeps trying to insert into the full atlas at position 0 instead of the new at `atlas.len() - 1`. With this simple fix a stack-overflow is prevented because the new atlas is inserted as the first element, which then will be used correctly for loading new glyphs. This fixes alacritty/issues/842 and might also solve alacritty/issues/914 (I wasn't able to reproduce this with the latest master).
Previously there were two separate but intended-to-be-identical implementations. Now the two implementations simply delegate to a single, shared method. This should help correctness issues in the future.
|
I confirm that it fixes the issue 👍 P.S. I played with holding Ctrl +/- and found a silly issue, posting just in case you can immediately think of a simple fix, otherwise I guess it's not worth looking into:
|
|
@maximbaz would you mind filing another issue for that? We can fix this with a change in our book keeping. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When an atlas is full and the
insertcall fails, a new atlas should becreated. This is the current strategy, however the atlas is put at the
end of the vector, but the
current_atlasindex is set to 0, instead ofthe last element. This leads to a recursion where it keeps trying to
insert into the full atlas at position 0 instead of the new at
atlas.len() - 1.With this simple fix a stack-overflow is prevented because the new atlas
is inserted as the first element, which then will be used correctly for
loading new glyphs.
This fixes /issues/842 and might also solve
/issues/914 (I wasn't able to reproduce this with the
latest master). Also fixes #898.