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 onDrawn callback #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tomhorak21
Copy link
Contributor

In some situations, it is necessary to react every time the canvas was updated (e.g., for some post processing). Therefore, this PR introduces a new init parameter onDrawn taking a callback function that is triggered every time the canvas was redrawn.

The callback get's triggered within the logDrawn method.
Small adjustment when logDrawn is called inside ssvg were made.

Copy link
Owner

@michaschwab michaschwab left a comment

Choose a reason for hiding this comment

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

Nice! Why are you adding a block of code that calls updateCanvas after a timeout?

@@ -159,7 +163,14 @@ export default class SSVG {
if(this.renderer.updatePropertiesFromQueue) {
this.renderer.updatePropertiesFromQueue(queue);
}

if(Object.keys(queue).length === 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this block of code needed?

Copy link
Contributor Author

@tomhorak21 tomhorak21 Sep 25, 2019

Choose a reason for hiding this comment

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

It's basically the same if-condition as used for the worker updating (line 154-158). Here, this ensures that this.renderer.draw() as well as this.logDrawn() are only called when something changed and not on every frame.

Copy link
Owner

Choose a reason for hiding this comment

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

I still don't understand it though. I do get that we use the same code for the worker, but the worker uses a different scheduling mechanism than the same thread method. As you can see in lines 70-80, for the version without the worker, updateCanvas is already continuously being called for every frame.

As far as I can see, what you are doing here is changing the scheduling mechanism for the non-worker method of ssvg. Is this actually related to what this pull request is trying to do? How is it related?

@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<project version="4">
<component name="ProjectTasksOptions">
<TaskOptions isEnabled="true">
Copy link
Owner

Choose a reason for hiding this comment

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

Could you remove this change please? This file should probably not be in the repo in the first place, but let's remove this change from this changelist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops, sorry. I've reverted it now.

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