LivelyKernel / lively.next

This is the repository of the lively.next project (https://lively-next.org).
MIT License
63 stars 16 forks source link

Error with Document Consistency Check #913

Open rickmcgeer opened 1 year ago

rickmcgeer commented 1 year ago

Describe the bug When I click on an item to get a Halo, this message pops up:

Error: Consistency check of document revealed 1 error:
Sum of child Height is not Height of node (leaf, rows: 0-7 size: 8 299.0625x249.49999999999997): 249.5 != 249

    at Document.Document_consistencyCheck_ (http://127.0.0.1:9011/lively.morphic/text/document.js:2565:23)
    at NewSceneGraphTree.Text_consistencyCheck_ (http://127.0.0.1:9011/lively.morphic/text/morph.js:3641:38)
    at eval (http://127.0.0.1:9011/lively.morphic/text/morph.js:1328:24)
    at ChangeManager.ChangeManager_groupChangesWhile_ (http://127.0.0.1:9011/lively.morphic/changes.js:416:17)
    at NewSceneGraphTree.Morph_groupChangesWhile_ (http://127.0.0.1:9011/lively.morphic/morph.js:647:45)
    at ChangeManager.ChangeManager_addMethodCallChangeDoing_ (http://127.0.0.1:9011/lively.morphic/changes.js:378:21)
    at NewSceneGraphTree.Morph_addMethodCallChangeDoing_ (http://127.0.0.1:9011/lively.morphic/morph.js:641:45)
    at NewSceneGraphTree.Text_replace_ (http://127.0.0.1:9011/lively.morphic/text/morph.js:1289:20)
    at eval (http://127.0.0.1:9011/lively.components/tree.js:250:26)
    at ChangeManager.ChangeManager_doWithValueChangeMeta_ (http://127.0.0.1:9011/lively.morphic/changes.js:356:23)

I traced this error to:

$world.execCommand("open browser", {moduleName: "text/document.js", packageName: "lively.morphic", });

And found this code in the method consistencyCheck

     const sumChildrenHeight = arr.sum(arr.pluck(children, 'height'));
    num.roundTo(sumChildrenHeight, 1) - num.roundTo(height, 1));

    if (num.roundTo(sumChildrenHeight, 1) != num.roundTo(height, 1)) { report.push({ error: `Sum of child Height is not Height of ${this}: ${sumChildrenHeight} != ${num.roundTo(height, 1)}` }); }

The summed height and height differed by 0.5. To fix this, I changed the code to:

     const sumChildrenHeight = arr.sum(arr.pluck(children, 'height'));
     const heightDiff = Math.abs(num.roundTo(sumChildrenHeight, 1) - num.roundTo(height, 1));

    if (heightDiff > 1) { report.push({ error: `Sum of child Height is not Height of ${this}: ${sumChildrenHeight} != ${num.roundTo(height, 1)}` }); }

which seemed to eliminate the problem. I can submit a PR on this if desired. Reproduced on both mac and windows.

Setup

Version: 5cecb8d2030617b92320cefc9ea4e63439e36630

linusha commented 1 year ago

@rickmcgeer what display resolution are you using? Do you have scaling factors active in your display settings?

linusha commented 1 year ago

@merryman what do you think? I suspect this is a resolution specific rounding problem and would think the suggested fix is good. Not sure how we can make sure of that though?

merryman commented 1 year ago

The thing is, I encounter this issue every once in a while. I believe this is due to some combination of scaling of text in conjunction with measuring, but I am not entirely sure. That being said I have been experimenting with rounding as suggested by @rickmcgeer but in my experienced it caused other issues with the document being inconsistent. Maybe we should wait this out until the new font measuring and the text renderer are revised and then see?