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

add animationFrameProvider map option #8131

Closed
wants to merge 1 commit into from
Closed

Conversation

ansis
Copy link
Contributor

@ansis ansis commented Apr 8, 2019

This option allows the user to implement requestAnimationFrame and cancelAnimationFrame in order to synchronize map rendering with their external rendering. While it is generally recommended to let the browser schedule map rendering frames this may be useful for synchronizing with external content in some cases (fix #7893).

This could also be useful for synchronizing multiple maps.

const animationFrameProvider = {
    requestAnimationFrame: fn => { ... },
    cancelAnimationFrame: handle => { ... }
};
const map = new Map({
    animationFrameProvider: animationFrameProvider,
    ...
});

As an alternative to synchronous render frames

#7893 explains the reasons why synchronous rendering is necessary in some cases. An animationFrameProvider option seemed preferable to implementing a map.synchronousRepaint() because:

  • it prevents all renders that aren't directly triggered by use (instead of just allowing renders to be triggered by you)
  • it could be harder to abuse
class SynchronizedFrameProvider {
    constructor() {
        this._requests = [];
        this._nextHandle = 0;
    }

    requestAnimationFrame(fn) {
        this._requests.push({
            handle: this._nextHandle++,
            fn
        });
    }

    cancelAnimationFrame(handle) {
        this._requests = this._requests.filter(req => req.handle !== handle);
    }

    // called by you to trigger synchronous rendering
    dispatchFrame() {
        // clear array before dispatching to handle case where user calls
        // `requestAnimationFrame` from within the dispatched function.
        const requests = this._requests;
        this._requests = [];

        const now = performance.now();
        for (const req of requests) {
            req.fn(now);
        }
    }
}

const provider = new SynchronizedFrameProvider();

const map = new Map({
    animationFrameProvider: provider
    ...
});

...

provider.dispatchFrame();

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page

@mourner @ryanhamley does this approach seem preferable? Or would it be better just implement a map.synchronousRender()?

This option allows the user to implement `requestAnimationFrame` and
`cancelAnimationFrame` in order to synchronize map rendering with their
external rendering.

fix #7893
@jahow
Copy link

jahow commented May 13, 2019

Hi!

The OpenLayers community is looking forward to this improvement as it will allow keeping an OpenLayers map and Mapbox in sync, thus giving people the best of both worlds! There is currently a working example but it uses an undocumented API, as was described in #7893 (comment)

Please let me know if there is anything we can do to help, inc. contribution proposal if necessary. Thanks a lot

@mourner
Copy link
Member

mourner commented Nov 8, 2019

It appears that there's no consensus on whether we should commit to this API — it doesn't solve the original issue that prompted it according to the author, and there are concerns over its ergonomics. Let's close the PR for now and re-evaluate the problem.

@mourner mourner closed this Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Synchronous redraw API
3 participants