-
Notifications
You must be signed in to change notification settings - Fork 75
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 video quads #745
Add video quads #745
Conversation
Also update some renderer documentation.
Don't test video in PhantomJS. Allow console.log messages to appear in tutorial test output.
Video quads are currently only supported by the canvas renderer. If the video is not ready, a preview color can be shown, but not a preview image. Added color quads to the canvas renderer. This made things easier to test. Improved some of the caching logic in quads to reduce reloading images and videos in some circumstances. Added a utility method to invalidate one or all of the quad's caches. This removes using a private method in the quad example. Added code to defer rendering if something is busy. This prevents flashing when seeking on a playing video. Fixed an issue with the canvas transform on images and videos.
Add a video to the quads example. Add two video quad tutorials; one stand-alone quad and one quad positioned on a map.
This PR is broken into separate commits to try to make it easier to review. |
src/canvas/canvasRenderer.js
Outdated
@@ -111,6 +111,13 @@ var canvasRenderer = function (arg) { | |||
features = layer.features(), | |||
i; | |||
|
|||
for (i = 0; i < features.length; i += 1) { | |||
if (features[i]._delayRender()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be if (!features[i]._delayRender())? If delayRender is true which means you want to skip rendering or something I missed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling render()
reschedules the rendering to the next animation frame. The return
then stops it from rendering right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks perhaps adding a comment here might help but I am fine either way.
src/canvas/quadFeature.js
Outdated
this._renderImageQuads = function (context2d, map) { | ||
if (!m_quads.imgQuads.length) { | ||
this._renderImageAndVideoQuads = function (context2d, map) { | ||
if ((!m_quads.imgQuads || !m_quads.imgQuads.length) && (!m_quads.vidQuads || !m_quads.vidQuads.length)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we wrap this long line?
Update a minor spacing issue in the comments of the single video tutorial and update its thumbnail to match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great..just has 1-2 minor comments
src/quadFeature.js
Outdated
return m_quads; | ||
}; | ||
|
||
/** | ||
* Initialize | ||
* If the data has changed and caching has been used, one or all data items |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sentence seems incomplete or missing some words?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. thank you for addressing the comments.
LGTM
Video quads are currently only supported by the canvas renderer. If the video is not ready, a preview color can be shown, but not a preview image (to show a preview image, create an image quad at the same coordinates as the video quad).