Proposed AI changes adding support for fit-content, font-feature-settings, ::first-letter, ::first-line

[poire-z]

Following up here (so the technical bits are remembered and searchable in the right repo) on the discussion started at koreader/koreader#14848 and some changes made with the help of AI by @reuerendo:

Hi. Recently, I managed to implement several improvements in crengine that I really missed: support for things like fit-content, ::first-letter, ::first-line, and font-feature-settings: "ss##".
The problem is that I'm not a programmer, and all the changes were made using AI. I can't assess the quality of this code. I only know that everything works for me.
Perhaps one of the developers could explain to me how to propose my changes for inclusion in the official build? And is it even worth proposing such things without being a programmer?

It was like that: fit-content -> first-letter -> first-line -> sets. Each branch contains the previous ones

master...reuerendo:crengine:sets
(I have kept a backup of your changes in case you mess with or delete your branch, there are some interesting ideas.)

(Sorry about the long write-up, maybe you spent less time co-piloting the AI than I did writing all this :/)

General thoughts about AI work and submissions

I'm not certain you are not a programmer :) There are bits here and there that feel like you may have written them (some comments in cyrillic, some bits a bit clumsy that feel not AI :)).
But there is a sure thing: you (and probably most new contributors) can't know about all crengine pecularities. It's somehow fragile, and if there are stuff not done in a walk-along-these-pecularities way, it may generate lots of others unrelated future issues (ie. rendering hash mismatch causing a full re-rendering on each document re-opening, or full-reparsing when we change a style via styletweak) that I may not see yet and that I (not the AI) will have to solve.
The AI may get a better feeling about all these pecularities with the help of my comments added along the years, but it's possible I didn't add comments about all of these, or that the codebase being so huge, the AI can't really grasp the interrelations of everything. But it is surely better at that than any new-at-crengine human can be :)

So, a newcomer using AI can surely get things done and working. But along the way, the AI must have suggested various options (I see in the changes: // ::first-line (Variant A): affect line breaking/metrics. - so it must have proposed other variants and possible solutions. It probably came with pros/cons AI text, and you were able to decide for one of the variants. But I'm a bit unconfortable as a reviewer (and probably the person who knows the most about crengine) just seeing the final result with decisions made by a person that is not. May be the cons for the variant you chose were really a long-term problem, or the AI didn't see one - and may be the other variants that felt to you like not the one to chose were actually the most promising one, may be with some tweaks that I could have proposed to the AI if I were the one doing the prompts :)

In my single experience with coding with AI: #645, the AI initially proposed 4 options, all were complicated - I then just suggested some other way, and it turned its vision completely, and went with my idea. And along the way, there were 4 times where I had to refocus it on some way of doing it.
So, It feels we may get somewhere more easily and quicker with AI, but (except for simple things like your fit-content) it feels it would have to be driven by somebody with more experience with crengine (that is: me :/).

Just curious: can you tell us about what AI tool/model you used and how ? Was there many many prompts, or you just told it "Implement this" and let it do its thing - maybe testing it and mentionning issues, and letting it solve them ? Or were you more involved in undestanding the changes and you made some change yourself?.

About fit-content

This one is the less harmful change, it's just some new property named value. The lvstsheet parsing feels ok (except the AI needlessely filled all the parse_number_value() default parameters with their default value of false).
I haven't really looked if the handling in lvrend.cpp is per-specs and right, but I will if this is made a standalone PR.
So, feel free to make a PR for this one (a standalone branch that you make a PR with).

About font-feature-settings

The AI has probably made the most annoying thing right: the parsing :)
In the implementation, the result is handled as a string, with lots of lString8::empty_str that I don't really like (maybe there are no other solution, dunno, I would have liked asking the AI :))
Also, if we were to support that, maybe we should get rid of the font features bitmap I added to support font-variant, which is redundant and more restricted (only 32 bits, so only 31 possible options, while having a string gives unlimited and support for unknown Opentype tags). So, it would need some thinking and possibly a rewrite of the font-variant support. And see what the specs says about interactions/precedence/inheritance between font-feature-settings and font-variant (the AI can probably tell us about that :)).
So, I'd rather you not pursue on this one.
Maybe I'll have a look in the coming months.

Just asking: why do you feel the need for that one? You have real books using these that don't have a font-variant equivalent (I guess a good publisher would use them for older eReader software that don't know about font-feature-settings).

About ::first-letter

The AI went with mimicking support for pseudo elements ::before and ::after, which feel like the most obvious solution.
But there is a fundamental flaw: it's modifying the DOM, wrapping the first letter in a new node, so cutting the first text node.
koreader/koreader#9142 (comment)

But even for :first-letter, it's complicated if there are inline nodes and the first letter is in some nested inline node [You or the AI solved that one]. Moreover, then, any highlight/xpointer made later in that paragraph would have their index shifted by 1 (as the first letter is no longer in the same text node). Quite too complicated to give it a go, for something that is just cosmetics :/

But you or the AI has overlooked the second one, which is a blocker. If people on previous books have made some highlights, they will be shifted by one char. And it will also happen if a person toggle Embedded styles or add a cosmetic styletweak to add that ::first-letter: their highlights in a paragraph with ::first-letter will be messed up.
This is something that the solution should handle. I would have been curious to see if the AI in its suggestions/variants had mention that :) and if not, I would have told it to see what alternative solutions it would come up with.

Also, it feels that the parsing/extraction of what is the first letter (per-specs, bringing dots around that first letter, that you did) miss bringing any diacritics following that base letter.

Anyway, your AI was quite good as grasping all of lvrend and lvtinydom, and it even had the idea of going with ldomDocument::saveFirstLineStylesData() (similar with how we handle list items numbers), holding some stuff in some independant datastructures that are saved in the cache, something I would have never thought about ! It's some idea I will try to keep in the back of my mind as a possible helper for some problems. Even if it feels it may not be necessary for ::first-letter.

Noting some thoughts about a possible idea to investigate to support ::first-letter:
To not mess with text nodes, may be add some attribute in the parent node (or create an empty node) flagging it hasFirstLetter, and maybe store in it the indice of the text node where the first-letter stops. Then, on rendering, build some "generated content" (like list items bullets and numbers, or pseudoelement ::before content) with that "first letter" chars. Render that with the style of the ::first-letter (so, need to create that empty node, like for ::before). Then, in lvtextfm, check if there is the attribute hasFirstLetter, and if yes, when measuring/rendering that text node, skip the first chars before that indice just as if there were not there in the text node.
This wouldn't break/shift highlights later in the paragraph, may be just highlights including that first letter would just not get that first letter part of the highlight (as it is generated content), which could be considered a cosmetic issue.

So, I'd rather you not pursue on this one. Maybe I'll have a look in the coming months.

About ::first-line

Your AI had some very clever idea I never thought of :)

        // ::first-line (Variant A): affect line breaking/metrics.
        // We first apply the overlay to the whole paragraph to let the existing
        // line-breaking logic determine the exact first line end with correct widths.
        // Right after the first line is emitted, we restore base metrics for the rest.
        bool firstLineOverlayActive = false;
        LVArray<src_text_fragment_t*> base_srcs;
        base_srcs.reserve(m_length);

Instead of bothering with a wrapping node or a generated content node (like it did for :first-letter) and the complexity to know up to which char or inline node (that will end that first line) we should wrap (and we could not even wrap if we are in the middle of a nested inline node), just have this handled by the text rendering code: copy all the text fragments (lower level datastructure than text nodes), apply to them the ::first-line font style (not sure how it handles different original styles, ie. bits in regular, bits in italic in the original text, and the first-line adding only font-weight, do we keep the italic?), measure all the paragraph text size with that font, render it (so, the first-line gets that style), and when done with the first line, restore the original fragments, re-measure the text, and go along rendering the next lines with the original font ! Very clever ! (but quite ugly, but the ugliness is quite localized, so I could bear it - and the overhead of measuring twice the paragraph).

But the ::first-line element, per-specs, allows for other CSS properties https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Selectors/::first-line#allowable_properties like color, background-color, that this solution can't handle (as it can just tweak the properties stored in the src_text_fragment_t that are all mostly just about the font). I'm curious if the AI mentionned that to you when proposing the variants, do you remember ?
May be this limitation would be just fine, but it's something to discuss - and I would have liked to see the AI other variants, may be it had some ideas on how to support more of the properties (and I would have decided if the added complexity is worth it or not).

So, I'd rather you not pursue on this one. Maybe I'll have a look in the coming months.

[reuerendo]

Hi, thank you for your reply.

I'm not certain you are not a programmer :) There are bits here and there that feel like you may have written them (some comments in cyrillic, some bits a bit clumsy that feel not AI :)).

I didn't write a single line of those you saw. Even cyrillic ones :) The only "code" that I understand and can write is HTML and CSS. Out of curiosity, I participated in group Python classes, but the group was quickly disbanded, as only two people remained in it (including me)

Just curious: can you tell us about what AI tool/model you used and how ? Was there many many prompts, or you just told it "Implement this" and let it do its thing - maybe testing it and mentionning issues, and letting it solve them ? Or were you more involved in undestanding the changes and you made some change yourself?.

In the case of crengine, I started with VS Code, but the copilot in this application works very poorly. I couldn't get the fit-content to work using VS code, so I switched to Cursor. The Cursor was able to complete this task, but he couldn't handle the next one - ::first-letter. So I decided to try Windsurf. All the rest of the code was written by Windsurf.

I understand, of course, that all these applications use basically the same LLM models. However, I see a huge difference in how they handle tasks. I don't know why. For me, the Windsurf with the GPT-5.2 Low Reasoning model works best.

As for prompts, I typically use a structure like: Study the project. Support for "something" needs to be added. Create a plan for implementing "something," then develop all the necessary code.

Then the AI ​​works, sometimes clarifying something. Sometimes it suggests options (as you noticed). Then I test what he wrote and complain about it not working :) Sometimes I have some idea of ​​how something should work, and then I propose it. For example, such a simple thing as the formula for calculating the height of the drop cap for the initial-letter property. I added this feature yesterday, I think you didn't see it.

fit-content

Could you write a few words about how to make a PR? Just click Contribute -> Open pull request in the fit-content branch?

font-feature-settings

Just asking: why do you feel the need for that one? You have real books using these that don't have a font-variant equivalent (I guess a good publisher would use them for older eReader software that don't know about font-feature-settings).

Correct me if I'm wrong, but you can't use OpenType Stylistic Sets (ss##) with font-variant-alternates. At first, I asked the AI ​​to implement this particular property, but nothing worked. Then I checked it in the browser, and it doesn't work. That is, font-feature-settings: "ss02"; is applied, but font-variant-alternates: styleset(ss02); is not.

If you need to know why I need support for Stylistic Sets at all: this property is quite important for languages ​​that do not use the Latin alphabet. For example, it has to be used in professional fonts such as Garamond Premier Pro, Minion 3, and others to make the appearance of cyrillic letters less archaic (why the font creators made the archaic character set the default value, I don’t understand).

Perhaps, as a person who used to layout paper books, I am overly meticulous about typography :)

I hope I didn't miss any questions :)
Thank you again

[Frenzie]

For example, it has to be used in professional fonts such as Garamond Premier Pro, Minion 3, and others to make the appearance of cyrillic letters less archaic

I think it might also play a role in making letters look Chinese vs Japanese, but don't quote me on that.

[poire-z]

For example, such a simple thing as the formula for calculating the height of the drop cap for the initial-letter property. I added this feature yesterday, I think you didn't see it.

No, I didn't see it. reuerendo/crengine@4f1bf91...reuerendo:crengine:initial-letter
(Fortunately, it depends on your implementation of ::first-letter that can't go in, so I don't have to read it :) But keep it around.)

You also have some tweak to regularizeRegisteredFontsWeights: reuerendo@cbe4a90

All this is a bit overwhelming :/
Nothing against you (you were honest and asked it it was ok, and you give me a chance to think about all this), but I need to write this :):
I haven't read anything about this (except maybe about the curl author complaining about AI bug submissions), but the ease of having AI write code is (or will become) a burden to maintainers.
It takes a few hours of serial-fun to copilot the AI to write many things - and it takes more time for maintainers to play at "find the fault" on the plain final result. At least, when somebody has spilled his own blood trying to humanly write code, it's alright for the maintainer to reward the pain with his own painful review. When it's so easy with AI, it's quite unfair to the maintainers :)
But it would also be unfair to reject AI submissions on principle,, as both parties's goal is to improve the project...
So, here we are - more future pain for maintainers :/

fit-content

Could you write a few words about how to make a PR? Just click Contribute -> Open pull request in the fit-content branch?

You'd need to start on a new branch sync'ed with our own master, and re-apply your changes, making sure they are standalone and work/compile fine (must be easy as fit-content was your first work on your big branch, you'd just need to checkout the last commit for the feature, create a new branch at this point, may be rebase these commits into a single one, re-read it to be sure it's clean and there's no noise).
Then indeed, you push your new branch to github, and you open a pull request for it. You write a bit about why you need that feature in real-life context (that is, the added code should be worth the required review and future maintenance - and it's not just some random property addition rarely seen in real books), you declare it was written by AI, and your skills (you already told all that, but let's have that as a reference) - and if you have kept some history, share the AI prompts and main responses (might be not much for fit-content and probably useless, but that would help reviewing more complex changes).

Correct me if I'm wrong, but you can't use OpenType Stylistic Sets (ss##) with font-variant-alternates. At first, I asked the AI ​​to implement this particular property, but nothing worked. Then I checked it in the browser, and it doesn't work. That is, font-feature-settings: "ss02"; is applied, but font-variant-alternates: styleset(ss02); is not.

I have no idea, I can't correct you :)

If you need to know why I need support for Stylistic Sets at all: this property is quite important for languages ​​that do not use the Latin alphabet. For example, it has to be used in professional fonts such as Garamond Premier Pro, Minion 3, and others to make the appearance of cyrillic letters less archaic (why the font creators made the archaic character set the default value, I don’t understand).

As an alternative solution, I guess you could hack your preferred fonts (with fontforge, or https://github.com/fonttools/fonttools, maintained by some people who did Harfbuzz that we do use) so that one stylistic set is make the default.

Perhaps, as a person who used to layout paper books, I am overly meticulous about typography :)

That's good to hear :) that you have some background. I hope you appreciate the work that has already been made to go towards good typography :)

[poire-z]

I think it might also play a role in making letters look Chinese vs Japanese, but don't quote me on that.

That should be handled by our Typography language, and the language tag we provide to Harfbuzz.
Then, there are - mostly for Japanese I think - additional Opentype tweaks to get glyph variants as it seem to have changed over the years:

crengine/crengine/src/lvstsheet.cpp

Lines 3946 to 3952 in 35ccd5c

	else if ( parse_eastasian && substr_icompare("simplified", decl) ) features |= LFNT_OT_FEATURES_P_SMPL;
	else if ( parse_eastasian && substr_icompare("traditional", decl) ) features |= LFNT_OT_FEATURES_P_TRAD;
	else if ( parse_eastasian && substr_icompare("full-width", decl) ) features |= LFNT_OT_FEATURES_P_FWID;
	else if ( parse_eastasian && substr_icompare("proportional-width", decl) ) features |= LFNT_OT_FEATURES_P_PWID;
	else if ( parse_eastasian && substr_icompare("jis78", decl) ) features |= LFNT_OT_FEATURES_P_JP78;
	else if ( parse_eastasian && substr_icompare("jis83", decl) ) features |= LFNT_OT_FEATURES_P_JP83;
	else if ( parse_eastasian && substr_icompare("jis04", decl) ) features |= LFNT_OT_FEATURES_P_JP04;

I haven't heard about the cyrillic variations before today, above :) I guess it didn't make it into having its own OpenType tag, and they rely on stylesets, so I guess that without generic names, one has to know about the font and the sets it implement, your prefered variant may be ss01 with one font, and ss02 on another :/ so it feels a bit less practical as one would need style tweaks when switching fonts (except for publishers that ship their own fonts and know the stylesets, but they may as well ship a font or a subset with the proper style by default, so it works on all ereaders - unless it's really a user choice, but then again, the user will have to try all the sets, without knowing the shipped font has the sets...)

[reuerendo]

You also have some tweak to regularizeRegisteredFontsWeights

I haven't tested it yet, I don't think you should waste your time on it.

I haven't heard about the cyrillic variations before today, above :) I guess it didn't make it into having its own OpenType tag, and they rely on stylesets, so I guess that without generic names, one has to know about the font and the sets it implement, your prefered variant may be ss01 with one font, and ss02 on another

You're right. This is a "pro" feature, so to speak :) For those who use their own CSS file.

I hope you appreciate the work that has already been made to go towards good typography

Oh, I appreciate that. You can't even imagine how much it annoyed me, for example, the inability to set a top margin for headings in Cool Reader for PocketBook, back when it still existed :)

For me, in terms of typography, KOreader only lacks those properties that I tried to implement.

[Commodore64user]

I haven't heard about the cyrillic variations before today

Surprising given the fact that crengine originates from the motherland ;)

[poire-z]

Noting some thoughts about a possible idea to investigate to support ::first-letter:
To not mess with text nodes, may be add some attribute in the parent node (or create an empty node) flagging it hasFirstLetter, and maybe store in it the indice of the text node where the first-letter stops. Then, on rendering, build some "generated content" (like list items bullets and numbers, or pseudoelement ::before content) with that "first letter" chars. Render that with the style of the ::first-letter (so, need to create that empty node, like for ::before). Then, in lvtextfm, check if there is the attribute hasFirstLetter, and if yes, when measuring/rendering that text node, skip the first chars before that indice just as if there were not there in the text node.
This wouldn't break/shift highlights later in the paragraph, may be just highlights including that first letter would just not get that first letter part of the highlight (as it is generated content), which could be considered a cosmetic issue.

Well, I think we may get support for ::first-letter :)
I spent a few hours thinking about it on sunday, looking at the code on how/where/when we could have that happening. Lots of dead-end ideas, until one lightbulb idea happened, that may work.
(I may have actually forgot this quoted text I had written only a few days before :/ but this is what I went towards.)
It would need quite a few steps, some copy & paste & adapt, so I decided I'll try once more to get github copilot to have a go at it. Which I did in poire-z#2 .

The AI was quite fine to work with, it followed my directions, I needed to correct it a few times.
It did help me finding a solution to something I missed in my idea and seemed like a dead-end poire-z#2 (comment).
It did find and describe well some solution poire-z#2 (comment)
It did simplify stuff when I asked poire-z#2 (comment).
It helps at appreciating the changes and risks when having some idea poire-z#2 (comment)
It did "hallucinate" once, giving an explanation (with bold bits and expressing no doubt) that is totally wrong poire-z#2 (comment).
It's quite ok to work with, although I guess it would reply with "You're absolutely right!" to whatever I may suggest :) (And it got me to the point that when it replies only with just "Fixed in commit", I get the feeling that it may be upset as beeing less cheerful, and I feel guilty :/).

So, quite some good experience, when you know where and how you want to go, and take it by the hand :)
(But I may have written more English that I would have written code if I had done it myself...)

Still some little tweaks to do (and comments to rewrite/add) before I can PR all that.

[reuerendo]

Glad to hear it :)
I was bored and yesterday I tasked the AI ​​to rewrite the ::first-letter implementation and used your comment as a promt. As a result, the pseudo-element is displayed as <p><pseudoElem FirsrLetter="S"/>Sample</p>. It doesn't cut out the letter, it just doesn't show it.

In general, it is not surprising that in the hands of someone who knows what to do, AI works better

[Frenzie]

It's quite ok to work with, although I guess it would reply with "You're absolutely right!" to whatever I may suggest :)

The newer version GPT-5.2 is supposedly less sycophantic. I think that means it'll just say "you're wrong!" to whatever you may suggest if it reaches that conclusion. ;-D

[poire-z]

I've read there was a new Claude Sonnet 4.6 that was super more clever than 4.5, so I decided to throw at it the task of implementing CSS ::first-line.
Its first implementation was quite limited poire-z#4 (comment) , so I gave him a few points to take into account, and go think again :)

Then, it did thought again - I could see the output in the private "Agent task" view on Github, and it was like James Joyce meets Woody Allen meets myself, some stream of consciousness, lots of really good understanding, and real logical ideas pursued but that ended up nowhere, complains about a chicken-and-egg problem :)... some sample:

[...] Modifying function signature... I'm considering a cleaner approach: format the paragraph twice—once with first-line styles applied to capture first-line metrics and breaks, then again normally for subsequent lines—then combine the first line's words from the first pass with the remaining words from the second pass. This avoids the complexity of partial re-measurement and font boundary tracking.
The challenge is that renderFinalBlock() is tightly coupled to the node's existing styles, so applying different styles for each pass would require either modifying the node temporarily (which isn't thread-safe), threading style overrides through the entire call chain, or creating a separate code path entirely. I suspect poire-z has something simpler in mind based on his comment about proposing ideas later—let me look more carefully at what he's describing about the paragraph container inheriting first-line styles.
He's essentially saying the pseudoElement acts as both a style holder and a signal that first-line handling is needed, but the real question is how to propagate those styles to the inline content correctly. I should be direct in my reply: acknowledge that the current approach of replacing fonts at draw time is fundamentally wrong because it ignores layout metrics, explain the two viable solutions I see (per-fragment overrides computed during rendering, or two-pass formatting), and ask about his architectural ideas rather than proposing a full implementation right now—he's clearly looking for collaborative analysis first. The challenge is distinguishing whether a fragment's weight matches the paragraph's because it was explicitly set or just inherited—if it's inherited, the first-line delta should apply, but if it's explicit, it shouldn't. Since the cached style doesn't track this distinction[...] (sharing the full outputs as an epub, 40 pages... Implement CSS --first-line pseudo-element support.zip)

after 10mn of that, Github decided to end its misery :) and kill it.
I asked it to "try again": same stuff - different ideas, but same mood, and after 10mn Github killed it again.

I then switched to Sonnet 4.5, which responded quickly (poire-z#4 (comment)):

[...] All seem complex. I'm ready to implement whichever approach you suggest, or hear your ideas on how to solve this.

Anyway, I had a few ideas to try, it built the groundwork (CSS parsing and element cloning), and I was able to continue trying a few ideas, and actually finding a quite clean and simple one. Outlined in the top post.
So, we may get support for ::first-line.

There are some bits spread out all around that change node->getText()/getAttribute()/... to use new node->getEffectiveText()/getEffectiveAttribute()/... method versions to fetch stuff from the original if on a clonedNode, that I'm not super happy that I'll have to see and decide which one to use in the future when adding new stuff.... but well :/
I looked among my 1000+ books, and I found only a single one that uses ::first-letter - and I found 0 that uses ::first-line.
It it really worth having ? (I'm quite proud of the solution, so I'm fine not abandonning it, even if I may regret it in the future.)
It can be quite pretty:

[Frenzie]

The end result looks great!

Incidentally, I've experimented a bit with the also new GPT-5.3 to fairly satisfactory results. Perhaps there are some AI instructions vis-à-vis crengine that we should put in the repo regarding common mistakes it makes? They'd go in .github/copilot-instructions.md.

[Commodore64user]

Perhaps there are some AI instructions vis-à-vis crengine that we should put in the repo regarding common mistakes it makes? They'd go in .github/copilot-instructions.md.

1. Do not delete the codebase ;)

::first-line.

yay