[l10n] Remove libraqm workarounds and enforce libraqm dependency

[alvroble]

Description

This PR implements the clean up of libraqm-related code by removing workarounds and ensuring proper libraqm support is enforced.

Changes Made

Add libraqm in dev build instructions (b24ca4b)

Modified raspberry_pi_os_build_instructions.md to include libraqm-dev in the required system packages
Changed the apt install command from:

sudo apt install -y build-essential zlib1g-dev libncurses5-dev libgdbm-dev libnss3-dev openssl libssl-dev libreadline-dev libffi-dev wget libsqlite3-dev

to:

sudo apt install -y build-essential zlib1g-dev libncurses5-dev libgdbm-dev libnss3-dev openssl libssl-dev libreadline-dev libffi-dev wget libsqlite3-dev libraqm-dev

This ensures Raspberry Pi OS dev environment have the necessary libraqm support.

Remove "libraqm not supported" tweaks (430f0cb)

Cleaned up text measurement workarounds in components.py:

• Removing fudge factor adjustments that compensated for imprecise text width calculations without libraqm.
• Removing compensating adjustment later in the code.

Fixed screenshot generator in generator.py:

• Removed the libraqm disable patch.
• Added libraqm validation check in test_generate_all.

This pull request is categorized as a:

• New feature
• Bug fix
• Code refactor
• Documentation
• Other

Checklist

• I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

• No, I’m a fool
• Yes
• N/A

I have tested this PR on the following platforms/os:

• Raspberry Pi OS Manual Build
• SeedSigner OS on a Pi0/Pi0W board
• Other

Bonus track!!

It seems the OP_RETURN problem from #762 gets solved with this modifications, although still with a bigger font size.

[kdmukai]

I know I suggested putting this here, but now that I think about it, it probably wouldn't hurt to have this check in the main code.

If the check is in, say, the Controller initialization, then it would cover both use cases: on-device check and when running the screenshot generator.

Or is that overkill? The vast majority of usage will be via a release image which we would already be guaranteeing that it's on board. It would really just be to catch any misconfigured dev setups.

I'm leaning: it's 53% useful to test for it all the time, 47% I dunno if it's necessary.

[alvroble]

I think that once we commit to supporting libraqm everywhere (dev and production), it’s reasonable for the Controller to fail if the dependency isn’t present. After all, it’s a functional requirement. Though, every developer would need to make sure they have it installed locally, and that’s added friction.

But on the other hand, if we rely on libraqm for proper behavior and start removing fallbacks or conditional adjustments from the code, then inconsistent behavior between setups becomes a real problem. In that sense, it does make sense to enforce the check in the Controller even if it’s slightly overkill for release images.

[kdmukai]

Ooh... we have a couple of if Settings.HOSTNAME == Settings.SEEDSIGNER_OS: checks. We could isolate the HAVE_RAQM test to just when we're NOT running SeedSigner OS.

Although... if we ever broke libraqm support in SeedSigner OS, a totally generalized check would catch that immediately.

Yeah, okay, never mind: I'd still say the check should run in all environments.

[alvroble]

So... I'm a bit hesitant to modify controller.py, I understand that we should do this check in the configure_instance method. But I'm not sure if we should make the whole application fail because of it.

[kdmukai]

It seems the OP_RETURN problem from #762 gets solved with this modifications, although still with a bigger font size.

I guess that's probably not too surprising since the HAVE_RAQM == False hacks to guesstimate the screen space intentionally overestimated the space required. HAVE_RAQM == True gives us precise bounding box calcs so if something should fit, it will fit!

[kdmukai]

Pending the one possible change mentioned above, I'm an easy:

untested ACK.

There are five HAVE_RAQM mentions in the code and this addresses (removes) all 5.

I would expect every screenshot to now have basically imperceptible differences (e.g. subtle letter spacing within words) or more obvious changes to line breaks due to different spacing calcs. I suppose it is possible that a screen might look worse if it was optimized around the imprecise calcs (e.g. something was off by 2 so we applied a +2 adjustment that is now undesirable).

So a careful look at all screenshots is on my TODO list before I can totally consider this ready to merge.

[newtonick]

@alvroble I'm having trouble passing the libraqm install check in tests/screenshot_generator/generator.py. When I run pytest tests/screenshot_generator/generator.py --locale en I get this error:

locale = 'en', target_locale = 'en'

    @pytest.mark.parametrize("locale", [x for x, y in SettingsConstants.get_detected_languages()])
    def test_generate_all(locale, target_locale):
        """
        `target_locale` is a fixture created in conftest.py via the `--locale` command line arg.

        Optionally skips all other locales.
        """
        if target_locale and locale != target_locale:
            pytest.skip(f"Skipping {locale}")

        if not ImageFont.core.HAVE_RAQM:
            # We can't generate pixel-perfect screenshots that match what gets rendered on
            # the device if we don't have libraqm.
>           pytest.fail("libraqm is not installed.")
E           Failed: libraqm is not installed.

tests/screenshot_generator/generator.py:62: Failed

I've completed the apt install of libraqm-dev and then completely re-compiled/install python3.10. I'm not sure what else to do. Any recommendations?

[alvroble]

@newtonick the only constraint is that you must install libraqm-dev before Pillow, but if you recompiled Python after installing libraqm that is fulfilled and I don't know what could be happening... to be sure, is it True or False the output of this command in the Python interpreter?

from PIL import features
features.check("raqm")

If not, maybe I'd check dependencies, are these packages installed when you install libraqm?
libfreetype6-dev, libharfbuzz-dev, libfribidi-dev

[newtonick]

@newtonick the only constraint is that you must install libraqm-dev before Pillow, but if you recompiled Python after installing libraqm that is fulfilled and I don't know what could be happening... to be sure, is it True or False the output of this command in the Python interpreter?

from PIL import features
features.check("raqm")

If not, maybe I'd check dependencies, are these packages installed when you install libraqm? libfreetype6-dev, libharfbuzz-dev, libfribidi-dev

Very helpful, thank you. I ended up running python3 -m pip uninstall Pillow and then python3 -m pip install --upgrade --force-reinstall --no-cache-dir Pillow==10.0.1. This resolved the issue. After that, features.check("raqm") returned True.

[newtonick]

ACK and tested to the best of my ability.

I also support adding a check to Controller for raqm support in PIL but I causing the application to fail without it could be a step to far IMO. I think it would be appropriate to add a logger message in a future PR. A message written out via logger when the Controller is first initialized would at least provide some heads up information to a future developer who might have missed the dependency when building.