Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Illustrations scale #937

Open
rgaudin opened this issue Nov 1, 2024 · 6 comments
Open

Support Illustrations scale #937

rgaudin opened this issue Nov 1, 2024 · 6 comments

Comments

@rgaudin
Copy link
Member

rgaudin commented Nov 1, 2024

While developing libzim7, we updated the spec to introduce Illustrations which are metadata for which dimensions and scale are stored as part of the metadata entry path.

We decided then that we wanted to store a scale so applications dont have to re-implement this on their own.

Unfortunately, to cut some corners and reach the release, we decided that libzim implementation will be deferred as the urgent need what supporting the favicon scenario through illustrations and that required a single size and scale.

To this day, it is still not possible to neither store nor retrieve an illustration at a scale other than 1 (~~ via the Illustration API)

void Creator::addIllustration(unsigned int size, std::unique_ptr<ContentProvider> provider)
{
checkError();
addMetadata(Formatter() << "Illustration_" << size << "x" << size << "@1",
std::move(provider), "image/png");

libzim/src/archive.cpp

Lines 157 to 159 in cdcbb6c

Item Archive::getIllustrationItem(unsigned int size) const {
auto r = m_impl->findx('M', Formatter() << "Illustration_" << size << "x"
<< size << "@" << 1);

I would now want to see this implemented.

@kelson42 kelson42 added this to the 9.3.0 milestone Nov 1, 2024
@kelson42
Copy link
Contributor

kelson42 commented Nov 2, 2024

@rgaudin Totaly in favour of fixing this quickly, but what would be the concrete use case (just linking an issue here is enough)?

@veloman-yunkan Doable easily?

@rgaudin
Copy link
Member Author

rgaudin commented Nov 2, 2024

Use case is displaying graphics correctly on devices which screen has higher density of pixels.

The devicePixelRatio is reported by the device (hence its name) and informs web browsers' media queries. Main usage is to toggle which bitmap image gets displayed based on the end-user's screen (to avoid auto up-scaling or down-scaling).

In our case, the illustration is currently a single 48x48@1 PNG. Basically a 48x48px image.

The idea behind our scale param is to store for example another illustration sourced from 96x96px PNG and store it as 48x48@2 (and a 144x144px one @3).
This way, readers can fetch and display the same illustration entry, only specifying the scale (using the DPR) and get the appropriate pixel-density in the image for it to look best.

This can be circumvented of course by storing 96x96@1 or 144x144@1 for instance but then the readers all need to query all illustration sizes and do this scale computation themselves.

Also, given Illustration_ is a single metadata only specified by size (and scale), it can be semantically wrong to assume that 96x96@1 is an higher density of the 48x48@1 illustration. We want to be able to store different images for other usages like covers, or anything else.

@benoit74
Copy link

benoit74 commented Nov 3, 2024

On Retina displays, having illustrations adapted to the DPR 2 is a big game changer in terms of look and feel. Anything not adapted really looks blurry since everything else is adapted to the Retina DPR of 2, and most eyes (not all of course) are able to see the difference. From my experience, most people don't know about the technical details, but they can feel something is awkward / broken when only DPR 1 is available.

Strongly in favor of implementing this quickly, of course, as everything related to the look and feel of Kiwix / openZIM.

@kelson42
Copy link
Contributor

kelson42 commented Jan 1, 2025

@benoit74 @rgaudin @veloman-yunkan I wonder if we should limit or standardize the possible values for the "dpr" argument. AFAIK the HTML/CSS engine can deal with whatever values given, but still it might be a good idea to not allow anything and for example limit to the 1, 2 or 3 values?

@rgaudin
Copy link
Member Author

rgaudin commented Jan 2, 2025

Readers/users will have to implement some matching code anyway since devicePixelRatio is a float (even though it often returns an int).
What benefit would there be in limiting this to a few values?

@benoit74
Copy link

benoit74 commented Jan 6, 2025

No strong opinion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants