Skip to content

feat: buffer map results and move to non-blocking #174

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

Merged
merged 3 commits into from
May 6, 2025

Conversation

milafrerichs
Copy link
Contributor

Description

use requestAnimationFrame to handle the parsing and updating of the map in between renderes of the browser
also use buffering to add as much points as possible

Relaxed Issue

#90

use requestAnimationFrame to handle the parsing and updating of the map
in between renderes of the browser
also use buffering to add as much points as possible
Copy link
Member

@rubensworks rubensworks left a comment

Choose a reason for hiding this comment

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

Nice work!


// If the given result contains geospatial data, show it on the map
_handleGeospatialResult: function (bindings) {
_process_map_data: function () {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_process_map_data: function () {
_processMapData: function () {

if (!this._mapResultsBuffer) this._mapResultsBuffer = [];
this._mapResultsBuffer.push(bindings);

requestAnimationFrame(this._process_map_data.bind(this));
Copy link
Member

Choose a reason for hiding this comment

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

We probably need a flag here to call requestAnimationFrame only once. After the callback is invoked, the flag can be reset, to allow future requestAnimationFrame calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, something like this:

      if (this._mapResultsBuffer.length > 0) {
        if (!this._processingScheduled) {
          this._processingScheduled = true;
          requestAnimationFrame(this._processMapData.bind(this));
        }
      }

Copy link
Member

Choose a reason for hiding this comment

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

Yep

// Show map if it's not visible yet
if (!self.$mapWrapper.is(':visible'))
self.$mapWrapper.show();
if (this._mapResultsBuffer.length > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Since we just pushed something to the buffer, this length check can be removed.

@milafrerichs milafrerichs requested a review from rubensworks May 6, 2025 12:19
@rubensworks rubensworks merged commit 3b08336 into comunica:master May 6, 2025
18 checks passed
@rubensworks
Copy link
Member

Nice work @milafrerichs!

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.

2 participants