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

Improve Rectangle's handling of border and clipping #1988

Open
tronical opened this issue Dec 14, 2022 · 4 comments
Open

Improve Rectangle's handling of border and clipping #1988

tronical opened this issue Dec 14, 2022 · 4 comments
Labels
rfc Request for comments: proposals for changes

Comments

@tronical
Copy link
Member

The Rectangle element has a border-width and border-colour property, as well as a clip property. How the border of a rectangle is draw in relation to the background of the Rectangle, as well as how the clipping applies in relation to the border is different between different renderers. In this issue we'd like to outline the current differences and discuss what the correct behavior should be, so that we can fix all of the renderers to produce the same output.

Suppose the following test-case:

export Demo := Window {
    width: 500px;
    height: 500px;
    first := Rectangle {
        x: 10px;
        y: 10px;
        width: 400px;
        height: 400px;
        background: #0000ff;
        border-color: #ff00006f;
        border-width: 50px;
        border-radius: 20px;
        clip: true;

        second := Rectangle {
            background: green;
            x: 200px;
            y: 200px;
        }
    }

    third := Rectangle {
        border-color: black;
        border-width: 1px;
        x: 10px;
        y: 10px;
        width: 400px;
        height: 400px;
    }
}

The first rectangle's border and clipping is what we would like to discuss. The second rectangle serves the purpose of visualising the clipping with regards to the border, and the third rectangle shows the effective external bounds of the first rectangle.

The following screenshots show the "current" state as of commit aea216f:

FemtoVG:
Screenshot 2022-12-14 at 15 31 09

Skia:
Screenshot 2022-12-14 at 15 31 32

Qt:
Screenshot 2022-12-14 at 15 31 54

Software renderer:
Screenshot 2022-12-14 at 15 33 35

It's a known issue that the software renderer doesn't do rounded clipping and that the border radius has the wrong size.

The two choices we're wondering about and are seeking feedback on are:

Border and Background

FemtoVG, Skia, and Qt show a purple area, which shows that in these the background is drawn first and it extends into the area of the border.

The two possibilities that we're thinking of are

  • The background covers the entire area of the border as well, the entire border would be rendered purple.
  • The background does not overlap with the border, the entire border would be rendered red.

Border and Clipping

The green rectangle is sometimes clipped within the blue area, sometimes half-way within the border, and sometimes entirely on the outside.

Should the clip property on the rectangle clip children within the the border or within the bounds of the rectangle?

@tronical tronical added the rfc Request for comments: proposals for changes label Dec 14, 2022
@ogoffart
Copy link
Member

It's a known issue that the software renderer [...] border radius has the wrong size.

I checked and it has the right size, looking at your screenshot in an image editor, the border radius is about 40 pixels, which is matching the 20px set with a scale factor of 2.

It is, in fact all the other renderers which are wrong as the actual radius is increased with the border size

ogoffart added a commit that referenced this issue Jan 17, 2023
…order

The current implementation of the software renderer doesn't drawn the
background under the border, so if the border is translusent, we don't
see the background color.
This patch blend the color of the border with the coler of the
background.

Software renderer part of #1988  (minus clipping)
ogoffart added a commit that referenced this issue Jan 18, 2023
…order

The current implementation of the software renderer doesn't drawn the
background under the border, so if the border is translusent, we don't
see the background color.
This patch blend the color of the border with the coler of the
background.

Software renderer part of #1988  (minus clipping)
@ogoffart
Copy link
Member

We didn't reach conclusion yet, but i think:

  • The background should extend under the border (border should be entirely purple) , this is what CSS does.

As for the clip, it all depends on whether we want to apply #110. If we want to say that the default border-width is 1px, then the clip shouldn't take the border into account. But in a sense, i think it makes more sense to take the border into account.

@tronical
Copy link
Member Author

We didn't reach conclusion yet, but i think:

  • The background should extend under the border (border should be entirely purple) , this is what CSS does.

In the light of our target audience and the principle of least surprise, I, too, think that we should pick this option. That's also what you implemented in #2078, right?

As for the clip, it all depends on whether we want to apply #110. If we want to say that the default border-width is 1px, then the clip shouldn't take the border into account. But in a sense, i think it makes more sense to take the border into account.

Another option here would be to let clip take an enum instead. So instead of clip: true; and the user wondering: "Oh, does this include the border?" it could be clip: border-box and clip: content-box - similar to CSS.

ogoffart added a commit that referenced this issue Jan 18, 2023
…order

The current implementation of the software renderer doesn't drawn the
background under the border, so if the border is translusent, we don't
see the background color.
This patch blend the color of the border with the coler of the
background.

Software renderer part of #1988  (minus clipping)
ogoffart added a commit that referenced this issue Jan 18, 2023
Also fix the border radius to be the outer radius of the rectangle

Qt renderer part of #1988  (minus clipping)
ogoffart added a commit that referenced this issue Jan 18, 2023
…rs (#2079)

Also fix the border radius to be the outer radius of the rectangle

Qt renderer part of #1988  (minus clipping)
tronical added a commit that referenced this issue Jan 18, 2023
Don't rely on the pen not being set when drawing the background of a rectangle
with transparent border. On my machine with the test case the pen has some other
color from previous drawing (or Qt internal?).
tronical added a commit that referenced this issue Jan 18, 2023
Don't rely on the pen not being set when drawing the background of a rectangle
with transparent border. On my machine with the test case the pen has some other
color from previous drawing (or Qt internal?).
tronical added a commit that referenced this issue Jan 19, 2023
…#2079)

Also fix the border radius to be the outer radius of the rectangle

FemtoVG renderer part of #1988 (minus clipping)
tronical added a commit that referenced this issue Jan 19, 2023
…#2079)

Also fix the border radius to be the outer radius of the rectangle

FemtoVG renderer part of #1988 (minus clipping)
tronical added a commit that referenced this issue Jan 19, 2023
…#2079)

Also fix the border radius to be the outer radius of the rectangle

FemtoVG renderer part of #1988 (minus clipping)
tronical added a commit that referenced this issue Jan 19, 2023
…2079)

Also fix the border radius to be the outer radius of the rectangle

Skia renderer part of #1988 (minus clipping)
tronical added a commit that referenced this issue Jan 20, 2023
…2079) (#2086)

Also fix the border radius to be the outer radius of the rectangle

Skia renderer part of #1988 (minus clipping)
@ogoffart
Copy link
Member

Status in current master:

image

Remaining problems:

  • Femtovg seems to be almost right here (appart from the small artifact of the border "leaking" slightly some pink pixles)
  • Software renderer doesn't implement border clipping at all (tracked in Software renderer radius clipping #4176)
  • Skia's clip is not of the correct width
  • Same for Qt's clip, in addition, the border is rendered outside of the radius

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Request for comments: proposals for changes
Projects
None yet
Development

No branches or pull requests

2 participants