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

UI Styles: Implement 'border' styling #119

Closed
bryphe opened this issue Dec 19, 2018 · 7 comments · Fixed by #123
Closed

UI Styles: Implement 'border' styling #119

bryphe opened this issue Dec 19, 2018 · 7 comments · Fixed by #123
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@bryphe
Copy link
Member

bryphe commented Dec 19, 2018

To have a useful UI - we should have parity with what the browser supports for the border styles: https://developer.mozilla.org/en-US/docs/Web/CSS/border

We should decide on our API surface. Perhaps something like this?

  • 2-pixel red border around entire element: <view style={Style.make(~border=Border(Colors.red, 2), ())} />
  • 1-pixel blue border on the left and right: <view style={Style.make(~borderHorizontal=Border(Colors.blue, 1)} />

For the initial implementation, only implementing the solid style seems reasonable. We can always add other styles down the road - supporting the border-width and border-color are important, though!

There are a few things we'll have to do to support this

Part 1: Style properties

Part 2: Rendering quads

For the core border rendering, we need to render a quad that spans the width (in the case of border-top/border-bottom) or the height (in the case of border-left or border-right), and positioned correctly in relationship to the node's layout.

This logic can live in ViewNode, and it's quite similiar to the logic we have today for drawing the background - the only difference really is the color and dimensions of the quad.

https://github.com/bryphe/revery/blob/711a90a0b19af2f39dd984cb1570a1231e745365/src/UI/ViewNode.re#L22

Part 3: Rendering Junctions

The trickiest piece of this work that will need to be handled with the rendering is where borders meet up. For example, if I have a top-border and a left-border, we need to render triangles potentially where these meet. This is dependent on us having a triangle primitive in #120

For example, if I had a div like this:

<div style="border-left: 10px solid red; border-top: 20px solid yellow; width: 100px; height:100px;">

It would look something like this:
image

Once we get Part 3 completed, though, we'll have a pretty usable border story 💯

@bryphe bryphe added help wanted Extra attention is needed good first issue Good for newcomers labels Dec 19, 2018
@OhadRau
Copy link
Collaborator

OhadRau commented Dec 19, 2018

Going to try working on this now that I have a little more free time. Will update here as I go.

@bryphe
Copy link
Member Author

bryphe commented Dec 19, 2018

@OhadRau - awesome, thank you! Let me know if you need any help.

@OhadRau
Copy link
Collaborator

OhadRau commented Dec 19, 2018

One quick question, is there any specific order that the borders should be resolved in? For example, if I were to make <view style=Style.make(~border=Border.make(~width=1, ()), ~borderLeft=Border.make(~width=2, ()), ())/> would the correct way to resolve this be always applying the most specific border? Or should they overlap if multiple are provided?

@bryphe
Copy link
Member Author

bryphe commented Dec 19, 2018

One quick question, is there any specific order that the borders should be resolved in?

That is a good question! My expectation would be that the more-specific border overrides the previous settings - so in the <view style=Style.make(~border=Border.make(~width=1, ()), ~borderLeft=Border.make(~width=2, ()), ())/> case, we'd end up with a top/right/bottom border of width 1, and a left border of width 2.

@OhadRau
Copy link
Collaborator

OhadRau commented Dec 19, 2018

Perfect. One more question, for drawing the border quads should I treat all the borders as interior items in the ViewNode (i.e. the main quad starts at (leftWidth, topWidth) instead of (0, 0))? If that's the case my understanding is that I'll have to adjust all positions to be relative to the top left corner of the main quad. The other alternative would be rendering the borders at a negative position (not sure if this would cause problems).

@OhadRau
Copy link
Collaborator

OhadRau commented Dec 20, 2018

Went ahead and implemented triangle geometry. The way I'm calculating the texture coordinates is really bad, I just wasn't sure what the proper math was so I left it WIP.

I also finished rendering the borders: 2018-12-19-230648_800x600_scrot

Still not sure how to approach the inner coordinates for the contents of the view node, so let me know if there's a certain way I should go about it. I couldn't see any obvious way to tell the super class to render inside the actual view area (rather than the entire area including the border) but I might've missed something.

@bryphe
Copy link
Member Author

bryphe commented Dec 20, 2018

Perfect. One more question, for drawing the border quads should I treat all the borders as interior items in the ViewNode (i.e. the main quad starts at (leftWidth, topWidth) instead of (0, 0))? If that's the case my understanding is that I'll have to adjust all positions to be relative to the top left corner of the main quad. The other alternative would be rendering the borders at a negative position (not sure if this would cause problems).

Ah, this is what i had a question about on the PR (I guess this is the root cause for the text rendering on the border). What I'd recommend for this is to account for the borderLeft/borderTop sizes in the local transform, here:
https://github.com/bryphe/revery/blob/b3c4274f6b5ec20c6847751927f54ea5bab761f7/src/UI/Node.re#L31

In other words, where we add in the top/left position:

 let matrix = Mat4.create();
    Mat4.fromTranslation(
      matrix,
      Vec3.create(
        float_of_int(dimensions.left),
        float_of_int(dimensions.top),
        0.,
      ),
    );

We also account for the borderTop/borderLeft sizes.

This puts all the children in a space where the coordinate system for the interior starts at (0, 0), which will put the children in the correct place - seems like the simplest approach.

However, as you mentioned, this would necessitate rendering the top and left borders at a negative position - that should be fine since those negative values will get adjusted when we transform the element to screen space. I think it's a reasonable price to pay to keep the rest of the transform chain simple.

Hope that helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants