From c628ebda0ff406494ec559fc913a7cb430bac785 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 18 Nov 2019 16:25:38 +0100 Subject: [PATCH] LibHTML: Use floating point numbers throughout the layout tree --- Libraries/LibHTML/CSS/Length.h | 13 ++++++++---- Libraries/LibHTML/CSS/StyleProperties.cpp | 5 ++--- Libraries/LibHTML/CSS/StyleProperties.h | 2 +- Libraries/LibHTML/HtmlView.cpp | 7 ++++--- Libraries/LibHTML/Layout/BoxModelMetrics.h | 8 +++---- Libraries/LibHTML/Layout/LayoutBlock.cpp | 16 +++++++------- Libraries/LibHTML/Layout/LayoutBox.cpp | 8 +++---- Libraries/LibHTML/Layout/LayoutBox.h | 21 ++++++++++--------- Libraries/LibHTML/Layout/LayoutImage.cpp | 8 +++---- Libraries/LibHTML/Layout/LayoutListItem.cpp | 2 +- .../LibHTML/Layout/LayoutListItemMarker.cpp | 2 +- Libraries/LibHTML/Layout/LayoutNode.cpp | 6 +++--- Libraries/LibHTML/Layout/LayoutNode.h | 3 ++- Libraries/LibHTML/Layout/LayoutText.cpp | 8 +++---- Libraries/LibHTML/Parser/CSSParser.cpp | 2 +- 15 files changed, 59 insertions(+), 52 deletions(-) diff --git a/Libraries/LibHTML/CSS/Length.h b/Libraries/LibHTML/CSS/Length.h index c557df974f..ccaa9e6c45 100644 --- a/Libraries/LibHTML/CSS/Length.h +++ b/Libraries/LibHTML/CSS/Length.h @@ -15,21 +15,26 @@ public: , m_value(value) { } + Length(float value, Type type) + : m_type(type) + , m_value(value) + { + } ~Length() {} bool is_auto() const { return m_type == Type::Auto; } bool is_absolute() const { return m_type == Type::Absolute; } - int value() const { return m_value; } + float value() const { return m_value; } String to_string() const { if (is_auto()) return "[Length/auto]"; - return String::format("%d [Length/px]", m_value); + return String::format("%g [Length/px]", m_value); } - int to_px() const + float to_px() const { if (is_auto()) return 0; @@ -38,7 +43,7 @@ public: private: Type m_type { Type::Auto }; - int m_value { 0 }; + float m_value { 0 }; }; inline const LogStream& operator<<(const LogStream& stream, const Length& value) diff --git a/Libraries/LibHTML/CSS/StyleProperties.cpp b/Libraries/LibHTML/CSS/StyleProperties.cpp index 744e1eeac4..0f1abe6977 100644 --- a/Libraries/LibHTML/CSS/StyleProperties.cpp +++ b/Libraries/LibHTML/CSS/StyleProperties.cpp @@ -101,10 +101,9 @@ void StyleProperties::load_font() const FontCache::the().set({ font_family, font_weight }, *m_font); } -int StyleProperties::line_height() const +float StyleProperties::line_height() const { - // FIXME: Allow overriding the line-height. We currently default to 140% which seems to look nice. - return (int)(font().glyph_height() * 1.4f); + return (float)font().glyph_height() * 1.4f; } bool StyleProperties::operator==(const StyleProperties& other) const diff --git a/Libraries/LibHTML/CSS/StyleProperties.h b/Libraries/LibHTML/CSS/StyleProperties.h index 4f0221bd39..5475d0dce2 100644 --- a/Libraries/LibHTML/CSS/StyleProperties.h +++ b/Libraries/LibHTML/CSS/StyleProperties.h @@ -32,7 +32,7 @@ public: return *m_font; } - int line_height() const; + float line_height() const; bool operator==(const StyleProperties&) const; bool operator!=(const StyleProperties& other) const { return !(*this == other); } diff --git a/Libraries/LibHTML/HtmlView.cpp b/Libraries/LibHTML/HtmlView.cpp index 9b2f88054a..50f5d34525 100644 --- a/Libraries/LibHTML/HtmlView.cpp +++ b/Libraries/LibHTML/HtmlView.cpp @@ -85,14 +85,14 @@ void HtmlView::layout_and_sync_size() main_frame().set_size(available_size()); document()->layout(); - set_content_size(layout_root()->size()); + set_content_size(enclosing_int_rect(layout_root()->rect()).size()); // NOTE: If layout caused us to gain or lose scrollbars, we have to lay out again // since the scrollbars now take up some of the available space. if (had_vertical_scrollbar != vertical_scrollbar().is_visible() || had_horizontal_scrollbar != horizontal_scrollbar().is_visible()) { main_frame().set_size(available_size()); document()->layout(); - set_content_size(layout_root()->size()); + set_content_size(enclosing_int_rect(layout_root()->rect()).size()); } #ifdef HTML_DEBUG @@ -360,7 +360,8 @@ void HtmlView::scroll_to_anchor(const StringView& name) return; } auto& layout_node = *element->layout_node(); - scroll_into_view({ layout_node.box_type_agnostic_position(), visible_content_rect().size() }, true, true); + FloatRect float_rect { layout_node.box_type_agnostic_position(), { (float)visible_content_rect().width(), (float)visible_content_rect().height() } }; + scroll_into_view(enclosing_int_rect(float_rect), true, true); window()->set_override_cursor(GStandardCursor::None); } diff --git a/Libraries/LibHTML/Layout/BoxModelMetrics.h b/Libraries/LibHTML/Layout/BoxModelMetrics.h index 7ed7bcb790..1d8f9a1471 100644 --- a/Libraries/LibHTML/Layout/BoxModelMetrics.h +++ b/Libraries/LibHTML/Layout/BoxModelMetrics.h @@ -17,10 +17,10 @@ public: const LengthBox& border() const { return m_border; } struct PixelBox { - int top; - int right; - int bottom; - int left; + float top; + float right; + float bottom; + float left; }; PixelBox full_margin() const; diff --git a/Libraries/LibHTML/Layout/LayoutBlock.cpp b/Libraries/LibHTML/Layout/LayoutBlock.cpp index 15705d82c8..df883947d3 100644 --- a/Libraries/LibHTML/Layout/LayoutBlock.cpp +++ b/Libraries/LibHTML/Layout/LayoutBlock.cpp @@ -41,7 +41,7 @@ void LayoutBlock::layout() void LayoutBlock::layout_block_children() { ASSERT(!children_are_inline()); - int content_height = 0; + float content_height = 0; for_each_child([&](auto& child) { // FIXME: What should we do here? Something like a might have a bunch of useless text children.. if (child.is_inline()) @@ -66,8 +66,8 @@ void LayoutBlock::layout_inline_children() line_box.trim_trailing_whitespace(); } - int min_line_height = style().line_height(); - int content_height = 0; + float min_line_height = style().line_height(); + float content_height = 0; // FIXME: This should be done by the CSS parser! CSS::ValueID text_align = CSS::ValueID::Left; @@ -82,9 +82,9 @@ void LayoutBlock::layout_inline_children() text_align = CSS::ValueID::Justify; for (auto& line_box : m_line_boxes) { - int max_height = min_line_height; + float max_height = min_line_height; for (auto& fragment : line_box.fragments()) { - max_height = max(max_height, enclosing_int_rect(fragment.rect()).height()); + max_height = max(max_height, fragment.rect().height()); } float x_offset = x(); @@ -137,7 +137,7 @@ void LayoutBlock::layout_inline_children() } if (is(fragment.layout_node())) - const_cast(to(fragment.layout_node())).set_rect(enclosing_int_rect(fragment.rect())); + const_cast(to(fragment.layout_node())).set_rect(fragment.rect()); float final_line_box_width = 0; for (auto& fragment : line_box.fragments()) @@ -178,7 +178,7 @@ void LayoutBlock::compute_width() padding_left = style.length_or_fallback(CSS::PropertyID::PaddingLeft, zero_value); padding_right = style.length_or_fallback(CSS::PropertyID::PaddingRight, zero_value); - int total_px = 0; + float total_px = 0; for (auto& value : { margin_left, border_left, padding_left, width, padding_right, border_right, margin_right }) { total_px += value.to_px(); } @@ -275,7 +275,7 @@ void LayoutBlock::compute_position() box_model().padding().bottom = style.length_or_fallback(CSS::PropertyID::PaddingBottom, zero_value); rect().set_x(containing_block()->x() + box_model().margin().left.to_px() + box_model().border().left.to_px() + box_model().padding().left.to_px()); - int top_border = -1; + float top_border = -1; if (previous_sibling() != nullptr) { auto& previous_sibling_rect = previous_sibling()->rect(); auto& previous_sibling_style = previous_sibling()->box_model(); diff --git a/Libraries/LibHTML/Layout/LayoutBox.cpp b/Libraries/LibHTML/Layout/LayoutBox.cpp index d0a6497da1..8e06a5cce1 100644 --- a/Libraries/LibHTML/Layout/LayoutBox.cpp +++ b/Libraries/LibHTML/Layout/LayoutBox.cpp @@ -22,7 +22,7 @@ void LayoutBox::render(RenderingContext& context) #endif if (node() && document().inspected_node() == node()) - context.painter().draw_rect(m_rect, Color::Magenta); + context.painter().draw_rect(enclosing_int_rect(m_rect), Color::Magenta); Rect padded_rect; padded_rect.set_x(x() - box_model().padding().left.to_px()); @@ -51,7 +51,7 @@ void LayoutBox::render(RenderingContext& context) auto border_style_value = style().property(CSS::PropertyID::BorderTopStyle); if (border_width_value.has_value()) { - int border_width = border_width_value.value()->to_length().to_px(); + float border_width = border_width_value.value()->to_length().to_px(); Color border_color; if (border_color_value.has_value()) @@ -99,7 +99,7 @@ HitTestResult LayoutBox::hit_test(const Point& position) const // FIXME: It would be nice if we could confidently skip over hit testing // parts of the layout tree, but currently we can't just check // m_rect.contains() since inline text rects can't be trusted.. - HitTestResult result { m_rect.contains(position) ? this : nullptr }; + HitTestResult result { m_rect.contains(FloatPoint(position.x(), position.y())) ? this : nullptr }; for_each_child([&](auto& child) { auto child_result = child.hit_test(position); if (child_result.layout_node) @@ -114,7 +114,7 @@ void LayoutBox::set_needs_display() ASSERT(frame); if (!is_inline()) { - const_cast(frame)->set_needs_display(rect()); + const_cast(frame)->set_needs_display(enclosing_int_rect(rect())); return; } diff --git a/Libraries/LibHTML/Layout/LayoutBox.h b/Libraries/LibHTML/Layout/LayoutBox.h index 3fa0c294bf..f4d0a8ecc4 100644 --- a/Libraries/LibHTML/Layout/LayoutBox.h +++ b/Libraries/LibHTML/Layout/LayoutBox.h @@ -1,19 +1,20 @@ #pragma once +#include #include class LayoutBox : public LayoutNodeWithStyleAndBoxModelMetrics { public: - const Rect& rect() const { return m_rect; } - Rect& rect() { return m_rect; } - void set_rect(const Rect& rect) { m_rect = rect; } + const FloatRect& rect() const { return m_rect; } + FloatRect& rect() { return m_rect; } + void set_rect(const FloatRect& rect) { m_rect = rect; } - int x() const { return rect().x(); } - int y() const { return rect().y(); } - int width() const { return rect().width(); } - int height() const { return rect().height(); } - Size size() const { return rect().size(); } - Point position() const { return rect().location(); } + float x() const { return rect().x(); } + float y() const { return rect().y(); } + float width() const { return rect().width(); } + float height() const { return rect().height(); } + FloatSize size() const { return rect().size(); } + FloatPoint position() const { return rect().location(); } virtual HitTestResult hit_test(const Point& position) const override; virtual void set_needs_display() override; @@ -31,7 +32,7 @@ protected: private: virtual bool is_box() const override { return true; } - Rect m_rect; + FloatRect m_rect; }; template<> diff --git a/Libraries/LibHTML/Layout/LayoutImage.cpp b/Libraries/LibHTML/Layout/LayoutImage.cpp index 158b2d9bb9..e2cb32cd36 100644 --- a/Libraries/LibHTML/Layout/LayoutImage.cpp +++ b/Libraries/LibHTML/Layout/LayoutImage.cpp @@ -38,18 +38,18 @@ void LayoutImage::render(RenderingContext& context) return; // FIXME: This should be done at a different level. Also rect() does not include padding etc! - if (!context.viewport_rect().intersects(rect())) + if (!context.viewport_rect().intersects(enclosing_int_rect(rect()))) return; if (renders_as_alt_text()) { context.painter().set_font(Font::default_font()); - StylePainter::paint_frame(context.painter(), rect(), FrameShape::Container, FrameShadow::Sunken, 2); + StylePainter::paint_frame(context.painter(), enclosing_int_rect(rect()), FrameShape::Container, FrameShadow::Sunken, 2); auto alt = node().alt(); if (alt.is_empty()) alt = node().src(); - context.painter().draw_text(rect(), alt, TextAlignment::Center, style().color_or_fallback(CSS::PropertyID::Color, document(), Color::Black), TextElision::Right); + context.painter().draw_text(enclosing_int_rect(rect()), alt, TextAlignment::Center, style().color_or_fallback(CSS::PropertyID::Color, document(), Color::Black), TextElision::Right); } else { - context.painter().draw_scaled_bitmap(rect(), *node().bitmap(), node().bitmap()->rect()); + context.painter().draw_scaled_bitmap(enclosing_int_rect(rect()), *node().bitmap(), node().bitmap()->rect()); } LayoutReplaced::render(context); } diff --git a/Libraries/LibHTML/Layout/LayoutListItem.cpp b/Libraries/LibHTML/Layout/LayoutListItem.cpp index a0da9dc064..ebe08fdb3f 100644 --- a/Libraries/LibHTML/Layout/LayoutListItem.cpp +++ b/Libraries/LibHTML/Layout/LayoutListItem.cpp @@ -21,6 +21,6 @@ void LayoutListItem::layout() append_child(*m_marker); } - Rect marker_rect { x() - 8, y(), 4, height() }; + FloatRect marker_rect { x() - 8, y(), 4, height() }; m_marker->set_rect(marker_rect); } diff --git a/Libraries/LibHTML/Layout/LayoutListItemMarker.cpp b/Libraries/LibHTML/Layout/LayoutListItemMarker.cpp index 2cbfd1cc57..98ffc61e07 100644 --- a/Libraries/LibHTML/Layout/LayoutListItemMarker.cpp +++ b/Libraries/LibHTML/Layout/LayoutListItemMarker.cpp @@ -13,7 +13,7 @@ LayoutListItemMarker::~LayoutListItemMarker() void LayoutListItemMarker::render(RenderingContext& context) { Rect bullet_rect { 0, 0, 4, 4 }; - bullet_rect.center_within(rect()); + bullet_rect.center_within(enclosing_int_rect(rect())); // FIXME: It would be nicer to not have to go via the parent here to get our inherited style. context.painter().fill_rect(bullet_rect, parent()->style().color_or_fallback(CSS::PropertyID::Color, document(), Color::Black)); } diff --git a/Libraries/LibHTML/Layout/LayoutNode.cpp b/Libraries/LibHTML/Layout/LayoutNode.cpp index b1ed18b827..43b9610cca 100644 --- a/Libraries/LibHTML/Layout/LayoutNode.cpp +++ b/Libraries/LibHTML/Layout/LayoutNode.cpp @@ -109,16 +109,16 @@ void LayoutNode::set_needs_display() } } -Point LayoutNode::box_type_agnostic_position() const +FloatPoint LayoutNode::box_type_agnostic_position() const { if (is_box()) return to(*this).position(); ASSERT(is_inline()); - Point position; + FloatPoint position; if (auto* block = containing_block()) { block->for_each_fragment([&](auto& fragment) { if (&fragment.layout_node() == this || is_ancestor_of(fragment.layout_node())) { - position = enclosing_int_rect(fragment.rect()).location(); + position = fragment.rect().location(); return IterationDecision::Break; } return IterationDecision::Continue; diff --git a/Libraries/LibHTML/Layout/LayoutNode.h b/Libraries/LibHTML/Layout/LayoutNode.h index f4e2181068..3602405a45 100644 --- a/Libraries/LibHTML/Layout/LayoutNode.h +++ b/Libraries/LibHTML/Layout/LayoutNode.h @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -108,7 +109,7 @@ public: template T* first_ancestor_of_type(); - Point box_type_agnostic_position() const; + FloatPoint box_type_agnostic_position() const; protected: explicit LayoutNode(const Node*); diff --git a/Libraries/LibHTML/Layout/LayoutText.cpp b/Libraries/LibHTML/Layout/LayoutText.cpp index bdf3c5e1c0..af49c185ac 100644 --- a/Libraries/LibHTML/Layout/LayoutText.cpp +++ b/Libraries/LibHTML/Layout/LayoutText.cpp @@ -126,13 +126,13 @@ void LayoutText::for_each_source_line(Callback callback) const void LayoutText::split_into_lines(LayoutBlock& container) { auto& font = style().font(); - int space_width = font.glyph_width(' ') + font.glyph_spacing(); - int line_height = style().line_height(); + float space_width = font.glyph_width(' ') + font.glyph_spacing(); + float line_height = style().line_height(); auto& line_boxes = container.line_boxes(); if (line_boxes.is_empty()) line_boxes.append(LineBox()); - int available_width = container.width() - line_boxes.last().width(); + float available_width = container.width() - line_boxes.last().width(); bool is_preformatted = style().string_or_fallback(CSS::PropertyID::WhiteSpace, "normal") == "pre"; if (is_preformatted) { @@ -176,7 +176,7 @@ void LayoutText::split_into_lines(LayoutBlock& container) for (int i = 0; i < words.size(); ++i) { auto& word = words[i]; - int word_width; + float word_width; bool is_whitespace = isspace(*word.view.begin()); if (is_whitespace) diff --git a/Libraries/LibHTML/Parser/CSSParser.cpp b/Libraries/LibHTML/Parser/CSSParser.cpp index 64bca0b3fe..7e09edb579 100644 --- a/Libraries/LibHTML/Parser/CSSParser.cpp +++ b/Libraries/LibHTML/Parser/CSSParser.cpp @@ -28,7 +28,7 @@ NonnullRefPtr parse_css_value(const StringView& view) char* endptr = nullptr; long value = strtol(String(view).characters(), &endptr, 10); if (endptr && ((!*endptr) || (endptr[0] == 'p' && endptr[1] == 'x' && endptr[2] == '\0'))) - return LengthStyleValue::create(Length(value, Length::Type::Absolute)); + return LengthStyleValue::create(Length((float)value, Length::Type::Absolute)); if (string == "inherit") return InheritStyleValue::create(); if (string == "initial")