On Sat, 22 Nov 2025 16:42:49 GMT, Marius Hanl <[email protected]> wrote:
>> The documentation on `Node.managed` property states >> >> Defines whether or not this node's layout will be managed by its parent. >> >> >> The layout of TextCell is not managed by its parent (`VFlow.content`), **by >> design**. >> >> The `VFlow` is the entity that does the layout, for a reason. It's a >> complicated thing, and multiple moving parts are connected (one example: it >> performs multiple layout cycles in the same layout pass when scrollbars >> appear/disappear during the layout pass, to avoid jumping and flicker - >> something we occasionally see with `ScrollPane` and `ListView`, see Note 1). >> >> Notes >> >> 1. I occasionally see the continuous flicker/layout when resizing the list >> of pages in the latest Monkey Tester, but so far I was unable to capture the >> exact conditions to create a reproducible test case (this is unrelated to >> this PR) > > Did you read what I wrote above? I made multiple suggestions as how we can > deal with the situation, even without making things managed again. > > To reiterate, what is the problem? > - We now have a dependency from `TextCell` to `VFlow`. Not explicitly, but > rather hidden, by searching all parents (which is also costly when doing that > often). I would prefer a clear separation of concerns > - This makes subclassing/extending harder. What happens, if I want to write > my own `VFlow`, but still use everything else, including `TextCell`? What if > I want to use `TextCell` for a component, that has no `VFlow`? What happens > if we find a `VFlow` from another component even? Because we used the > `TextCell` somewhere else? > - Other components made it clear how to use `managed` and `unamanged` nodes. > And how to bubble up a request still. Why do we want to make an exception > here? This is the first time we need to search the parent hierarchy > - Why not e.g. binding to the `needsLayoutProperty` from a `TextCell` from > within `VFlow` and just request the layout when set? > > Also another point: The test is green for me with the changes from: > https://github.com/openjdk/jfx/pull/1945 > So I would like to know, if maybe there was a bug even? I also had a similar concern as @Maran23. But, after looking it more closely, It seems to be acceptable and needed solution for this scenario. Few aspects: 1. Alternative approaches would be to save a reference to parent VFlow in each TextCell or a listener property. Which could be huge number, as there would be **n** TextCells (non-node embedding) under a VFlow. 2. The parent traversal is always fixed, i.e. only one time, as VFlow is immediate parent of a TextCell. 3. The re-layouts of VFlow are be limited, once a pulse. and at-least one is required. 3.1 even if multiple Node embedding TextCells get modified in a pulse. 3.2 even with multiple layout of a TextCell in a pulse Overall this seems safe, with fixed negligible impact on performance, as traversal happens only for the modified TextCell. So, it would be one traversal for modified node(very likely one node in a frame/pulse). and alternative approaches would increase memory consumption depending on number of TextCells. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1975#discussion_r2681053781
