Do not create a block frame for out-of-flow items only

[MDLC01]

Fixes #2297

This PR makes it so that, when a breakable block is broken into multiple frames, a frame containing only out-of-flow items (which correspond to place elements, counter updates, etc.) will move its items to the next frame if there is one.

Please review this carefully! I was not familiar with this part of the codebase when I started working on that, so please make sure I did not write anything stupid. All tests pass, so it should be ok, but there are still some things I'm not fully confident about (see comments). In particular, because this fix is implemented by modifying the way FlowElem is laid out, I think it may affect other things (although I could not think of any).

[laurmaedje]

Just removing the last frame seems like a strange thing to do. It probably happens to work because the next frame is too large to fit into the first region and the parent flow handles it, but generally a multi-region layouter should produce one frame per region.

One thing that might work is to mem::take the only-out-of-flow items at the start of finish_region to effectively create a completely empty frame and then, once that's done readd them to the current items. This way, you also wouldn't need a new field in the layouter.

[MDLC01]

Just removing the last frame seems like a strange thing to do.

I agree that completely removing frames is weird. The problem is that, if I push Frame::new(Size::zero(), FrameKind::Soft) after pop()-ing a frame, it adds an empty page in the last test in out-of-flow-in-block.

One thing that might work is to mem::take the only-out-of-flow items at the start of finish_region to effectively create a completely empty frame and then, once that's done readd them to the current items. This way, you also wouldn't need a new field in the layouter.

I'm not sure how to do that, because at at the start of finish_region, we no longer know whether the previous frame contained only out-of-flow elements (we would still need a boolean field, which I agree is better than cloning the elements).

[laurmaedje]

I'm not sure how to do that, because at at the start of finish_region, we no longer know whether the previous frame contained only out-of-flow elements (we would still need a boolean field, which I agree is better than cloning the elements).

What I meant is that rather than writing and then removing the frame with only out-of-flow elements, just don't create the frame at all, effectively deferring writing of the only out-of-flow elements. (Perhaps with the exception of the final finish_region call where we need to ensure that they are written.)

[MDLC01]

Sorry for the delayed response. This is indeed smarter, although I now have this problem where there is an additional blank page in the last test.

By removing the following lines, I can show the first frame even if it is empty (this part is unrelated to my PR, it is just a graphical thing). In the last test, the first frame is actually displaced in the second region, which is what causes the issue.

typst/crates/typst-library/src/layout/container.rs

Lines 431 to 433 in 987e97c

	if let [first, rest @ ..] = frames.as_slice() {
	skip = first.is_empty() && rest.iter().any(|frame| !frame.is_empty());
	}

I will try to rebase on main and understand why this happens this week, but I suspect this might be the consequence of another bug.

[MDLC01]

Turns out setting the frame size to self.initial instead of Size::zero() solved the issue of the last test not passing. Now all tests pass.

I will try to resolve the conflicts and solve the issue that frames with size zero are not necessarily out-of-flow, and then this will be ready to merge. In the meantime, I'm switching this PR to a draft.

[MDLC01]

I think this should now be safe to merge!

[laurmaedje]

Thank you, also for the patience!