Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Replace three maps/mutexes in GlyphAtlas with a single map and mutex #8199

Merged
merged 1 commit into from
Feb 28, 2017

Conversation

jfirebaugh
Copy link
Contributor

Fixes the GlyphAtlas equivalent of #6835.

@mention-bot
Copy link

@jfirebaugh, thanks for your PR! By analyzing this pull request, we identified @tmpsantos, @1ec5 and @artemp to be potential reviewers.

}

// FIXME: We lock all GlyphSets, but what we should
// really do is lock only the one we are returning.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Fixme isn't resolved, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but I don't think it'll ever be fixed in the way described here. Instead, we'll do #667.

@jfirebaugh jfirebaugh merged commit e3500c1 into master Feb 28, 2017
@jfirebaugh jfirebaugh deleted the refactor-glyph-atlas branch February 28, 2017 00:19
@tmpsantos
Copy link
Contributor

This causes platform/node/test/memory.test.js to deadlock on src/mbgl/text/glyph_pbf.cpp:120.

@tmpsantos
Copy link
Contributor

This causes platform/node/test/memory.test.js to deadlock on src/mbgl/text/glyph_pbf.cpp:120.

After some investigation, the cause seems to be related to the mock file source used in that test returning the reply synchronously.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants