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

Update docs to describe raster-fade-duration's effect on rendering videos #297

Merged

Conversation

neodescis
Copy link
Collaborator

@neodescis neodescis commented Aug 11, 2023

Partially addresses the documentation update requested in maplibre/maplibre-gl-js#2922. Will also open a PR in that repo to mention the property in video_source.ts.

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

@neodescis
Copy link
Collaborator Author

Now that I've made this change, I guess I should ask... Does this property affect video similarly in other things that use the spec, beyond GL JS?

@codecov-commenter
Copy link

Codecov Report

Patch and project coverage have no change.

Comparison is base (141ca39) 77.83% compared to head (f3fdf5f) 77.83%.
Report is 18 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #297   +/-   ##
=======================================
  Coverage   77.83%   77.83%           
=======================================
  Files         100      100           
  Lines        4105     4105           
  Branches     1176     1176           
=======================================
  Hits         3195     3195           
  Misses        910      910           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@neodescis neodescis changed the title Update docs to describe raster-fade-duration's effect on rendering vi… Update docs to describe raster-fade-duration's effect on rendering videos Aug 11, 2023
Copy link
Member

@HarelM HarelM left a comment

Choose a reason for hiding this comment

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

THANKS!

@HarelM
Copy link
Member

HarelM commented Aug 12, 2023

Like what?

@neodescis
Copy link
Collaborator Author

neodescis commented Aug 12, 2023

Like what?

What I mean is, do my new comments in the docs only apply to GL, or does native work the same way (with the fade in)? I've never used anything other than GL.

@HarelM
Copy link
Member

HarelM commented Aug 12, 2023

They shouldn't, the spec should be cross platform, but just in case:
Cc @louwers

Copy link
Collaborator

@louwers louwers left a comment

Choose a reason for hiding this comment

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

Since this source is not supported by MapLibre Native, it is clear that the description applies to GL JS only.

louwers

This comment was marked as resolved.

Copy link
Collaborator

@louwers louwers left a comment

Choose a reason for hiding this comment

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

Yes so video sources are not supported by Native, that means the addition is not relevant for Native, but it will be once videos are supported. Fine to merge I think.

@HarelM HarelM merged commit fcc156c into maplibre:main Aug 16, 2023
6 checks passed
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.

None yet

4 participants