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

[core] Refcount the view activations #5874

Merged
merged 1 commit into from
Aug 6, 2016
Merged

Conversation

tmpsantos
Copy link
Contributor

@tmpsantos tmpsantos commented Aug 5, 2016

Fixes #5836
Fixes #5721

@tmpsantos
Copy link
Contributor Author

👀 @kkaefer @ivovandongen @mikemorris

@tmpsantos
Copy link
Contributor Author

On android we still have to activate/deactivate the context because on some phones you have multiple contexts around.

But it is now "refcounted" so it supports nested invocations.

@tmpsantos tmpsantos force-pushed the tmpsantos-gl_ctx_active branch 2 times, most recently from 64dc189 to e7e49d9 Compare August 5, 2016 15:17
@tmpsantos tmpsantos changed the title [core] Do not activate/deactivate the view [core] Refcount the view activations Aug 5, 2016
@tmpsantos
Copy link
Contributor Author

Reducing the scope just for android, gonna be one week out and this could potentially have some unwanted side effects.

@ivovandongen ivovandongen merged commit 0c460a5 into master Aug 6, 2016
@ivovandongen ivovandongen deleted the tmpsantos-gl_ctx_active branch August 6, 2016 17:02
@kkaefer
Copy link
Member

kkaefer commented Aug 15, 2016

Why are you refcounting? Shouldn't it be something like "activate when not activated" and "deactivate when still active" and make all other operations no-ops? Rather than count the number of activations, is there a way to fix the underlying issue so that they aren't activated/deactivated in the first place?

@tmpsantos
Copy link
Contributor Author

Why are you refcounting? Shouldn't it be something like "activate when not activated" and "deactivate when still active" and make all other operations no-ops? Rather than count the number of activations, is there a way to fix the underlying issue so that they aren't activated/deactivated in the first place?

That would not work. The buggy code was:

Render
    Activate
        Signal (mapped to OnLoaded on Android)
            Client calls AddLayer
                Activate
                Do some GL
                Deactivate
    Do some more GL
    Deactivate

So the Do some more GL would have a deactivated context. Another way of fixing I tried was not calling activate/deactivate at all and assume the context is always valid inside mbgl-core. We would transfer this responsibility to client/platform. What do you think?

@kkaefer
Copy link
Member

kkaefer commented Aug 17, 2016

Well, we could only call Activate/Deactivate after AddLayer when the context is not active?

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.

3 participants