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

Refactor timing: Code review issues, masks refactoring in progress #367

Merged
merged 16 commits into from
Dec 17, 2015

Conversation

shamansir
Copy link
Contributor

No description provided.

@shamansir
Copy link
Contributor Author

@skavish please take a look

child.render(bctx, dt);
});

// FIXME: this should be performed one time for all masked elements!
mask.transform(mctx);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@skavish I am uncertain in this part. The matrix is calculated once, but current bounds affect the mask canvas size. Is it safe to move it to mask render cycle?

Copy link
Member

Choose a reason for hiding this comment

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

mask canvas size does not change during one rendering cycle. the masked elements and the mask itself are rendered into canvases the same size.

you need to draw all masked elements into a canvas first, then (after all masked elements are rendered) you need to render mask itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made a draft for this, not yet tested: refactor-timing...refactor-masks. I will ensure if order of rendering (masked elements before mask itself) is right later, but may be you'll see some pitfalls obvious for you.

Copy link
Member

Choose a reason for hiding this comment

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

I want to comment there, but I cannot comment on a diff, only on pullrequest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are welcome: #368 :)

@skavish skavish added the LGTM label Dec 17, 2015
shamansir added a commit that referenced this pull request Dec 17, 2015
Refactor timing: Code review issues, masks refactoring in progress
@shamansir shamansir merged commit 8eb2285 into develop Dec 17, 2015
@shamansir shamansir changed the title Refactor timing: Code review issues, masks refactoring Refactor timing: Code review issues, masks refactoring in progress Dec 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants