Skip to content

Enable processing pixel lengths as float #415

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Enable processing pixel lengths as float #415

wants to merge 3 commits into from

Conversation

DRKV333
Copy link

@DRKV333 DRKV333 commented Jul 1, 2025

Hey, this is still very much WIP, but I would like to get some feedback on it.

I've started work on converting the whole library to use float rather than int when calculating the length of things in pixels, to avoid compounding rounding errors. This is something users have requested a few times by now. The general plan was:

  • Introduce a new pixel_t typedef in types.h to represent pixel sizes, just pointing to int initially
  • Look at every usage if int everywhere, and if it represents a size in pixels, replace it with pixel_t
  • Add a preprocessor define, switchable through cmake, to have pixel_t point to float instead
  • Look at compiler warnings and fix any conversion issues

At this point I have looked through the code and replaced everything. I've also tested it with float and it does compile, but there are a few things to fix still.

I do have a few questions though on how to proceed:

Are you sure you would want to support both int and float pixel types?
It seems like this would be quite annoying. Both variants would have to be tested, or if one isn't tested it might break. I'm not sure what the benefit of int would be (other than backward compatibility maybe). A page would have to be several million pixels large before precision becomes a problem. The performance seems about the same from what I've measured.

How should the API be handled?
I think it would make the most sense to keep the pixels as float all the way until the document container, and then that can decide what to do with them. But then this would be a breaking change. From what I can tell the API surface is basically most things in the include folder, and a lot of that has changed. In theory it might be possible to keep having the API use int by duplicating a bunch of structures, but that would be very painful.
Right now, with pixel_t being an alias for int, there is no breakage, so keeping that the default and adding a switch could solve this problem.
Also there are some structures whose name doesn't make sense now, like typed_int and int_int_cache. These also always store pixel sizes so they now use pixel_t. But they are part of the API surface so renaming them would be breaking.

I've also replaced font size and media query related pixel values with pixel_t for consistency, though I'm not sure how necessary that is really.

As a demonstration consider an example similar to #14:

HTML 
<!DOCTYPE html>
<html>
    <head>
        <title>Hello</title>
        <style>
            .expected {
                position: absolute;
                height: 100px;
                width: 151px;
                top: 50px;
                background-color: red;
            }

            .test1 {
                position: relative;
                height: 100px;
                width: 50px;
                top: 10px;
                left: 33.3px;
                background-color: blue;
            }

            .test2 {
                position: relative;
                height: 90px;
                width: 56.9px;
                left: 60.8px;
                background-color: green;
            }
        </style>
    </head>
    <body>
        <div class="test1">
            <div class="test2">
            </div>
        </div>

        <div class="expected"></div>
    </body>
</html>

int:
image

float:
image

The right edge of the red box should line up with the right edge of the green box, but with int there is a noticeable gap. Using float fixes this.

Closes #47
Closes #14

@tordex
Copy link
Member

tordex commented Jul 1, 2025

This is very impressive work!

Are you sure you would want to support both int and float pixel types?

I think the float support is enough.

How should the API be handled?
I think it would make the most sense to keep the pixels as float all the way until the document container, and then that can decide what to do with them.

You are right, this is the only painless way. As for API break - the library is in beta status. API is changed frequently. The latest master is not compatible with the latest numerical version because of new features and removing legacy. It is OK.

Also there are some structures whose name doesn't make sense now, like typed_int and int_int_cache. These also always store pixel sizes so they now use pixel_t. But they are part of the API surface so renaming them would be breaking.

containing_block_context::typed_int is internal type. It can be renamed to typed_pixel for example.
int_int_cache - the same. Can be renamed to ```pixel_cache`` for example.
These names don't make sense now, agree.

It is interesting, how many tests will fail after switching to the floats?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature request: Make coordinate types configurable as float percentages are rounded down causing pixel offsets
2 participants
@DRKV333 @tordex