sile-typesetter / sile

The SILE Typesetter — Simon’s Improved Layout Engine
https://sile-typesetter.org
MIT License
1.61k stars 97 forks source link

Discretionaries in new liners #1984

Closed Omikhleia closed 5 months ago

Omikhleia commented 5 months ago

We went merging #1977 a bit too soon.

Edge case: image

Discretionaries still need more refactoring so as to avoid such cases (and ugly guesses everywhere they are used)

alerque commented 5 months ago

It's okay, this isn't a regression since multi-line effects like this were not even possible before. I'm okay with iterating on the develop branch as long as we're sure we're moving the bar forward, and we're not locked into supporting any given API shape until we release.

Do you have sources for this test case to add (even without proper expectations)?

Omikhleia commented 5 months ago

Do you have sources for this test case to add (even without proper expectations)?

Currently using a minimized "parbox" from parbox.sile after I had a sudden doubt, so I still have to craft a test that doesn't need such an overkill.

Omikhleia commented 5 months ago

A possible MWE

\begin[papersize=a7, class=resilient.book]{document}
\lua{
local class = SILE.documentState.documentClass
class:registerCommand("xredacted", function (options, content)
  local bs = SILE.measurement("0.9bs"):tonumber()
  local bsratio = 0.3 -- hard-coded here, should use font metrics
  SILE.typesetter:liner("xredacted", content,
    function (box, typesetter, line)
      local outputWidth = SU.rationWidth(box.width, box.width, line.ratio)
      local H = SU.max(box.height:tonumber(), (1 - bsratio) * bs)
      local D = SU.max(box.depth:tonumber(), bsratio * bs)
      local X = typesetter.frame.state.cursorX
      SILE.outputter:pushColor(SILE.color("gray"))
      SILE.outputter:drawRule(X, typesetter.frame.state.cursorY - H, outputWidth, H + D)
      SILE.outputter:popColor()
      -- MVE: Use the box width to advance position, rather
      -- than rely on internal content to move it.
      typesetter.frame:advanceWritingDirection(outputWidth)
    end
  )
end)
}
\language[main=pl]
% In liner
\xredacted{logi-zoty logi-zoty logi-zoty logi-zoty logi-zoty logi-zoty
logi-zoty logi-zoty logi-zoty}

% Vs. normal
logi-zoty logi-zoty logi-zoty logi-zoty logi-zoty logi-zoty
logi-zoty logi-zoty logi-zoty

\end{document}

Current = wrong, see blue lines

image

Fixed as per #1985 = correct

image

Analysis

With underline/striketrough etc., we don't use the box width, but rely on the inner content being output to move the frame cursor.

Here we used the target outputWidth to move the cursor (ignoring content): it proves the box's width is incorrect, due to not taking into account (on line 2) the discretionary. EDIT: on line 1, it's not a discretionary, it's the dash; but one line 2, the computation misses both the initial postbreak (repeated hyphen in Polish) and the final prebreak.

Omikhleia commented 5 months ago

(Sorry if that MWE is non-trivial + uses resilient, that was one quick way to get sile-x shim and test with 0.14.16)

alerque commented 5 months ago

Thanks for the MWE. I simplified it a bit more, stripped out some of the extra noise, and made it work (er, fail) with the plain class, then verified the PR made it work.