Skip to content

New CSS value system#196

Open
ananas-dev wants to merge 1 commit into
mainfrom
lufio/value
Open

New CSS value system#196
ananas-dev wants to merge 1 commit into
mainfrom
lufio/value

Conversation

@ananas-dev
Copy link
Copy Markdown
Member

No description provided.

Comment thread src/vaev-engine/values/line-width.cpp Outdated
Comment on lines 22 to 25
// FIXME: Remove
export constexpr Au THIN_VALUE = 1_au;
export constexpr Au MEDIUM_VALUE = 3_au;
export constexpr Au THICK_VALUE = 5_au;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not delete it then ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread src/vaev-engine/values/length.cpp Outdated
// 6. MARK: Distance Units: the <length> type
// https://drafts.csswg.org/css-values/#lengths

// export using Px = Distinct<f64, struct _PxTag>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread src/vaev-engine/values/length.cpp Outdated
Comment on lines +33 to +51
Px snapped() {
// 1. Assert: len is non-negative.
if (value() <= 0.0f) {
panic("border snapping negative length");
}

// 2. If len is an integer number of device pixels, do nothing.
if (Math::abs(value() - Math::round(value())) < Limits<f32>::EPSILON) {
return Px(Math::round(value()));
}

// 3. If len is greater than zero, but less than 1 device pixel, round len up to 1 device pixel.
if (value() <= 1.0f) {
return Px(1.0f);
}

// 4. If len is greater than 1 device pixel, round it down to the nearest integer number of device pixels.
return Px(Math::floor(value()));
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Missing const
  • Name is also too generic, should be something like snappedAsBorderWidth or something.
  • Good that you are doing that at value time but the layout code that did it too wasn't removed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the const and the naming.
I didn't see the snapping code in the layout, the length visitor of line-width only calls resolve(length)

Comment thread src/vaev-engine/values/media.cpp Outdated
Comment on lines +398 to +401
// FIXME:
// // https://drafts.csswg.org/mediaqueries/#typedef-mq-boolean
// export template <>
// struct _Computed<bool> : IdentityComputed<bool> {};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Comment thread src/vaev-engine/values/ratio.cpp Outdated
Comment on lines +24 to +26
auto operator==(Ratio const& other) const {
return Math::epsilonEq(num / deno, other.num / other.deno);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is epsilonEq really needed here ? Also not sure it's the right sema for operator==()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back to IEEE semantics since it doesn't make sense to compare aspect ratios by equality anyways.

Comment thread src/vaev-engine/values/percent.cpp Outdated
Comment on lines +38 to +39
// FIXME: Refactor during calc refactor to look something like this:
// LengthPercentage = Length | Percent | Calc
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not just length you can have percentage of other stuff ;)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I clarified the comment a bit

Comment thread src/vaev-engine/values/keywords.cpp Outdated
export template <StrLit K>
struct ValueParser<Keyword<K>> {
struct ValueTraits<Keyword<K>> {
// FIXME: Plz remove
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this is trying to say

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I didn't want default value traits on keywords (identity computed value etc..) but I changed my mind so I removed the comment.

Comment thread src/vaev-engine/values/insets.cpp Outdated
.col = valueFromComputed<Gap>(computed.col)
};
}
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a computed value property group, it doesn't make sens to have all the value boilerplate, the following should be fine.

export struct GapProps {
    Computed<Gap> row = Keywords::NORMAL;
    Computed<Gap> col = Keywords::NORMAL;

    void repr(Io::Emit& e) const {
        e("(gaps {} {})", row, col);
    }
};

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done !

Comment thread src/vaev-engine/values/keywords.cpp Outdated
Comment on lines +54 to +67
template <>
struct ValueTraits<Keywords::Auto> : DefaultValueTraits<Keywords::Auto> {
static Res<Keywords::Auto> parse(Cursor<Css::Sst>& c) {
if (c.ended())
return Error::invalidData("unexpected end of input");

if (c->token == Css::Token::ident("auto")) {
c.next();
return Ok(Keywords::Auto{});
}

return Error::invalidData("expected keyword");
}
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this use just the normal keyword parsing infra?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@ananas-dev ananas-dev force-pushed the lufio/value branch 6 times, most recently from 0eefb5b to 7478089 Compare May 8, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants