Skip to content

fix: avoid double application of a scale transform #1645

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 2 commits into from
May 27, 2023
Merged

Conversation

Fil
Copy link
Contributor

@Fil Fil commented May 27, 2023

when using a derived mark (such as a tip), a scale transform was applied twice

@Fil Fil requested a review from mbostock May 27, 2023 14:40
@Fil Fil changed the title avoid double application of a scale transform fix: avoid double application of a scale transform May 27, 2023
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Nice! This is almost exactly the solution I imagined too.

I renamed channel.transformed to channel.transform and inverted the meaning: it now means whether to apply the scale’s transform and defaults to true; if false, then the scale’s transform is bypassed. I think this is clearer if an initializer wants to declare that a channel should bypass the scale’s transform, and is similar to what we do for channel.filter. I also added it to the Channel interface.

@mbostock mbostock force-pushed the fil/double-transform branch from 3beb45f to 9870085 Compare May 27, 2023 17:21
@mbostock mbostock merged commit 9339c24 into main May 27, 2023
@mbostock mbostock deleted the fil/double-transform branch May 27, 2023 17:40
chaichontat pushed a commit to chaichontat/plot that referenced this pull request Jan 14, 2024
* avoid double application of a scale transform when using a derived mark (such as a tip)

* transform instead of transformed

---------

Co-authored-by: Mike Bostock <[email protected]>
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