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

Percentage padding not updated when the parent size changes #1626

Open
1 task done
Omxjep3434 opened this issue Mar 14, 2024 · 4 comments
Open
1 task done

Percentage padding not updated when the parent size changes #1626

Omxjep3434 opened this issue Mar 14, 2024 · 4 comments

Comments

@Omxjep3434
Copy link

Omxjep3434 commented Mar 14, 2024

Report

When a child has a percentage padding, it is initially correct in regard to the parent's size, but it is not updated in subsequent Yoga executions unless the child becomes dirty.

I am testing this with the Javascript Yoga npm package v2.0.1, but this may be an issue with the layout engine.

Issues and Steps to Reproduce

Here is some very basic code with console output to reproduce.

function percentPadding(yoga) {
    const config = yoga.Config.create();
    config.setPointScaleFactor(0);

    const parent = yoga.Node.createWithConfig(config);
    parent.setWidth(10);

    const child = yoga.Node.createWithConfig(config);
    child.setWidth(5);
    child.setPaddingPercent(Edge.Left, 50);

    parent.insertChild(child, 0);

    // On the first execution, the child's padding is as expected: 50% * 10 = 5.
    console.log("** Executing **");
    parent.calculateLayout(10, 10);
    console.log(`Child - padding left: ${child.getComputedPadding(Edge.Left)}`);

    // Now change the parent width to 5. The expected child padding is: 50% * 5 = 2.5.
    // However, we can see that the padding is still 5.
    parent.setWidth(5);
    console.log("** Executing **");
    parent.calculateLayout(10, 10);
    console.log(`Child - padding left: ${child.getComputedPadding(Edge.Left)}`);

    // If we make an arbitrary change, just to make the child dirty, the padding is now correct at 2.5
    child.setBorder(Edge.Top, 0);
    console.log("** Executing **");
    parent.calculateLayout(10, 10);
    console.log(`Child - padding left: ${child.getComputedPadding(Edge.Left)}`);
}

function main() {
    const yoga = await loadYoga();

    percentPadding(yoga);
}
@NickGerleman
Copy link
Contributor

Do you know if this same issue happen if the parent is not the tree root? Wondering if there might be some funkiness around setting a different root available space when triggering the layout, but also setting a definite width on the root.

@NickGerleman
Copy link
Contributor

Normally layout is reevaluated if available width/height constraints imposed by parent don’t change, and we’re laying out in same mode. But there have been plenty of bugs around here before.

The flex basis issue is a bit different because it has its own caching mechanism.

@Omxjep3434
Copy link
Author

I confirmed the issue happens if the parent isn't the root. I have slightly modified snippet here:

function percentPadding(yoga) {
    const config = yoga.Config.create();
    config.setPointScaleFactor(0);

    const root = yoga.Node.createWithConfig(config);
    root.setWidth(10);

    const parent = yoga.Node.createWithConfig(config);
    parent.setWidth(10);

    const child = yoga.Node.createWithConfig(config);
    child.setWidth(5);
    child.setPaddingPercent(Edge.Left, 50);

    parent.insertChild(child, 0);
    root.insertChild(parent, 0);

    // On the first execution, the child's padding is as expected: 50% * 10 = 5.
    console.log("** Executing **");
    root.calculateLayout(10, 10);
    console.log(`Child - padding left: ${child.getComputedPadding(Edge.Left)}`);

    // Now change the parent width to 5. The expected child padding is: 50% * 5 = 2.5.
    // However, we can see that the padding is still 5.
    parent.setWidth(5);
    console.log("** Executing **");
    root.calculateLayout(10, 10);
    console.log(`Child - padding left: ${child.getComputedPadding(Edge.Left)}`);

    // If we make an arbitrary change, just to make the child dirty, the padding is now correct at 2.5
    child.setBorder(Edge.Top, 0);
    console.log("** Executing **");
    root.calculateLayout(10, 10);
    console.log(`Child - padding left: ${child.getComputedPadding(Edge.Left)}`);
}

function main() {
    const yoga = await loadYoga();

    percentPadding(yoga);
}

@nicoburns
Copy link
Contributor

nicoburns commented Mar 21, 2024

@NickGerleman I believe the "available space" and the size for percentage resolution aren't necessarily always the same. For example, you may wish to size a child under a max-content constraint (unconstrained) but still resolve percentages. e.g. when sizing the main axis of flexbox items the "available space" is infinite but percentages should still resolve against the parent container's size if it's definite.

I believe the cache key should contain (separately) both:

  • the sizing constraint in each axis (which will be either an exact size constraint or else an "available space" constraint (either a definite pixel value, min-content, max-content, etc))
  • The percentage resolution sizes in each axis

I was going to post that this is what Taffy does, but although we're storing two values in the cache key (https://github.com/DioxusLabs/taffy/blob/main/src/tree/cache.rs#L11) we're grouping the non-exact sizing constraint with the percentage resolution size rather than the exact sizing constraint which I think is wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants