这是indexloc提供的服务，不要输入任何密码
Skip to content

Remove cache_mut, needs_measure and measure_node methods from LayoutTree trait #431

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 11, 2023
Merged

Remove cache_mut, needs_measure and measure_node methods from LayoutTree trait #431

merged 3 commits into from
Apr 11, 2023

Conversation

@nicoburns
Copy link
Collaborator

@nicoburns nicoburns commented Apr 10, 2023

Objective

The aim is again to simplify the LayoutTree trait by removing unncessary methods. In this case, it is achieved by making the generic compute/mod and leaf compute functions take &mut Taffy instead of &mut impl LayoutTree. This works because these methods are private, and are only used when using the built in Taffy tree anyway.

Context

Split out from #326

Feedback wanted

General PR review.

@nicoburns nicoburns added usability Make the library more comfortable to use breaking-change A change that breaks our public interface labels Apr 10, 2023
@nicoburns nicoburns requested review from TimJentzsch and Weibye April 10, 2023 21:51
This was referenced Apr 10, 2023
Closed
Merged
TimJentzsch
TimJentzsch approved these changes Apr 10, 2023
View reviewed changes
Copy link
Collaborator

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

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

LGTM.

I noticed that we seem to be using the terms "node", "key" and "node id" to refer to the same thing (a NodeId), perhaps in a future PR we should try to make the variable names more consistent here

@nicoburns
Copy link
Collaborator Author

nicoburns commented Apr 10, 2023 •
edited
Loading

LGTM.

I noticed that we seem to be using the terms "node", "key" and "node id" to refer to the same thing (a NodeId), perhaps in a future PR we should try to make the variable names more consistent here

key is different as it's a slotmap::DefaultKey, which we still need to use internally in the tree code (but not the compute code) as we're still using slotmaps for storage. But agree totally on node and node_id. I think we should be using node_id (or even just id) anytime the variable is a NodeId. But there's just a lot of places to update so it might takes a while to go through and update everything.

@nicoburns nicoburns merged commit 384b60b into DioxusLabs:main Apr 11, 2023
@nicoburns nicoburns deleted the remove-cache-and-measure-funcs-from-layout-tree-trait branch April 11, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Weibye Weibye Awaiting requested review from Weibye

1 more reviewer

@TimJentzsch TimJentzsch TimJentzsch approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking-change A change that breaks our public interface usability Make the library more comfortable to use

Projects

None yet

Milestone

0.4

Development

Successfully merging this pull request may close these issues.

2 participants

@nicoburns @TimJentzsch