Skip to content

Commit 713d906

Browse files
gaaclarkegaaclarke
authored
Started clamping scaled antialias lines size (#166149)
fixes flutter/flutter#165994 Now vertices and e value calculations are driven by the clamping that is happening in ComputeCorners. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent c068f67 commit 713d906

File tree

4 files changed

+128
-35
lines changed

4 files changed

+128
-35
lines changed

‎engine/src/flutter/impeller/entity/contents/line_contents.cc‎

Lines changed: 63 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,11 @@ std::shared_ptr<Texture> CreateCurveTexture(
4747
return CreateTexture(texture_descriptor, curve_data, context, "LineCurve");
4848
}
4949

50-
GeometryResult CreateGeometry(const ContentContext& renderer,
51-
const Entity& entity,
52-
RenderPass& pass,
53-
const Geometry* geometry) {
50+
std::pair<LineContents::EffectiveLineParameters, GeometryResult> CreateGeometry(
51+
const ContentContext& renderer,
52+
const Entity& entity,
53+
RenderPass& pass,
54+
const Geometry* geometry) {
5455
using PerVertexData = LineVertexShader::PerVertexData;
5556
const LineGeometry* line_geometry =
5657
static_cast<const LineGeometry*>(geometry);
@@ -59,7 +60,8 @@ GeometryResult CreateGeometry(const ContentContext& renderer,
5960
auto& host_buffer = renderer.GetTransientsBuffer();
6061

6162
size_t count = 4;
62-
fml::Status calculate_status;
63+
fml::StatusOr<LineContents::EffectiveLineParameters> calculate_status =
64+
LineContents::EffectiveLineParameters{.width = 0, .radius = 0};
6365
BufferView vertex_buffer = host_buffer.Emplace(
6466
count * sizeof(PerVertexData), alignof(PerVertexData),
6567
[line_geometry, &transform, &calculate_status](uint8_t* buffer) {
@@ -68,19 +70,24 @@ GeometryResult CreateGeometry(const ContentContext& renderer,
6870
vertices, line_geometry, transform);
6971
});
7072
if (!calculate_status.ok()) {
71-
return kEmptyResult;
73+
return std::make_pair(
74+
LineContents::EffectiveLineParameters{
75+
.width = line_geometry->GetWidth(),
76+
.radius = LineContents::kSampleRadius},
77+
kEmptyResult);
7278
}
7379

74-
return GeometryResult{
75-
.type = PrimitiveType::kTriangleStrip,
76-
.vertex_buffer =
77-
{
78-
.vertex_buffer = vertex_buffer,
79-
.vertex_count = count,
80-
.index_type = IndexType::kNone,
81-
},
82-
.transform = entity.GetShaderTransform(pass),
83-
};
80+
return std::make_pair(calculate_status.value(),
81+
GeometryResult{
82+
.type = PrimitiveType::kTriangleStrip,
83+
.vertex_buffer =
84+
{
85+
.vertex_buffer = vertex_buffer,
86+
.vertex_count = count,
87+
.index_type = IndexType::kNone,
88+
},
89+
.transform = entity.GetShaderTransform(pass),
90+
});
8491
}
8592

8693
struct LineInfo {
@@ -111,7 +118,6 @@ LineInfo CalculateLineInfo(Point p0, Point p1, Scalar width, Scalar radius) {
111118
1.0 + k * (p1.x * p1.x + p1.y * p1.y - p0.x * p1.x - p0.y * p1.y)),
112119
};
113120
}
114-
115121
} // namespace
116122

117123
const Scalar LineContents::kSampleRadius = 1.f;
@@ -136,6 +142,10 @@ bool LineContents::Render(const ContentContext& renderer,
136142
frag_info.color = color_;
137143

138144
Scalar scale = entity.GetTransform().GetMaxBasisLengthXY();
145+
146+
auto geometry_result =
147+
CreateGeometry(renderer, entity, pass, geometry_.get());
148+
139149
std::shared_ptr<Texture> curve_texture = CreateCurveTexture(
140150
geometry_->GetWidth(), kSampleRadius, scale, renderer.GetContext());
141151

@@ -161,7 +171,11 @@ bool LineContents::Render(const ContentContext& renderer,
161171
return true;
162172
},
163173
/*force_stencil=*/false,
164-
/*create_geom_callback=*/CreateGeometry);
174+
/*create_geom_callback=*/
175+
[geometry_result = std::move(geometry_result)](
176+
const ContentContext& renderer, const Entity& entity,
177+
RenderPass& pass,
178+
const Geometry* geometry) { return geometry_result.second; });
165179
}
166180

167181
std::optional<Rect> LineContents::GetCoverage(const Entity& entity) const {
@@ -185,20 +199,39 @@ std::vector<uint8_t> LineContents::CreateCurveData(Scalar width,
185199
return curve_data;
186200
}
187201

188-
fml::Status LineContents::CalculatePerVertex(
189-
LineVertexShader::PerVertexData* per_vertex,
190-
const LineGeometry* geometry,
191-
const Matrix& entity_transform) {
192-
Point corners[4];
202+
namespace {
203+
void ExpandLine(std::array<Point, 4>& corners, Point expansion) {
204+
Point along = (corners[1] - corners[0]).Normalize();
205+
Point across = (corners[2] - corners[0]).Normalize();
206+
corners[0] += -1 * (across * expansion.x) + -1 * (along * expansion.y);
207+
corners[1] += -1 * (across * expansion.x) + (along * expansion.y);
208+
corners[2] += (across * expansion.x) + -1 * (along * expansion.y);
209+
corners[3] += (across * expansion.x) + (along * expansion.y);
210+
}
211+
} // namespace
212+
213+
fml::StatusOr<LineContents::EffectiveLineParameters>
214+
LineContents::CalculatePerVertex(LineVertexShader::PerVertexData* per_vertex,
215+
const LineGeometry* geometry,
216+
const Matrix& entity_transform) {
217+
Scalar scale = entity_transform.GetMaxBasisLengthXY();
218+
std::array<Point, 4> corners;
219+
// Make sure we get kSampleRadius pixels to sample from.
220+
Scalar expand_size = std::max(kSampleRadius / scale, kSampleRadius);
193221
if (!LineGeometry::ComputeCorners(
194-
corners, entity_transform,
222+
corners.data(), entity_transform,
195223
/*extend_endpoints=*/geometry->GetCap() != Cap::kButt,
196-
geometry->GetP0(), geometry->GetP1(),
197-
geometry->GetWidth() + kSampleRadius * 2.0)) {
224+
geometry->GetP0(), geometry->GetP1(), geometry->GetWidth())) {
198225
return fml::Status(fml::StatusCode::kAborted, "No valid corners");
199226
}
200-
LineInfo line_info = CalculateLineInfo(geometry->GetP0(), geometry->GetP1(),
201-
geometry->GetWidth(), kSampleRadius);
227+
Scalar effective_line_width = std::fabsf((corners[2] - corners[0]).y);
228+
ExpandLine(corners, Point(expand_size, expand_size));
229+
Scalar padded_line_width = std::fabsf((corners[2] - corners[0]).y);
230+
Scalar effective_sample_radius =
231+
(padded_line_width - effective_line_width) / 2.f;
232+
LineInfo line_info =
233+
CalculateLineInfo(geometry->GetP0(), geometry->GetP1(),
234+
effective_line_width, effective_sample_radius);
202235
for (auto& corner : corners) {
203236
*per_vertex++ = {
204237
.position = corner,
@@ -209,6 +242,7 @@ fml::Status LineContents::CalculatePerVertex(
209242
};
210243
}
211244

212-
return {};
245+
return EffectiveLineParameters{.width = effective_line_width,
246+
.radius = effective_sample_radius};
213247
}
214248
} // namespace impeller

‎engine/src/flutter/impeller/entity/contents/line_contents.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,15 @@ class LineContents : public Contents {
1919
Scalar radius,
2020
Scalar scale);
2121

22-
static fml::Status CalculatePerVertex(
22+
struct EffectiveLineParameters {
23+
Scalar width;
24+
Scalar radius;
25+
};
26+
27+
/// Calculates the values needed for the vertex shader, per vertex.
28+
/// Returns the effective line parameters that are used. These differ from the
29+
/// ones provided by `geometry` when the line gets clamped for being too thin.
30+
static fml::StatusOr<EffectiveLineParameters> CalculatePerVertex(
2331
LineVertexShader::PerVertexData* per_vertex,
2432
const LineGeometry* geometry,
2533
const Matrix& entity_transform);

‎engine/src/flutter/impeller/entity/contents/line_contents_unittests.cc‎

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,21 @@ TEST(LineContents, CalculatePerVertex) {
6464
/*cap=*/Cap::kButt);
6565
Matrix transform;
6666

67-
fml::Status status =
67+
fml::StatusOr<LineContents::EffectiveLineParameters> status =
6868
LineContents::CalculatePerVertex(per_vertex, geometry.get(), transform);
6969
Scalar offset =
7070
(LineContents::kSampleRadius * 2.0 + geometry->GetWidth()) / 2.f;
7171
ASSERT_TRUE(status.ok());
72-
EXPECT_POINT_NEAR(per_vertex[0].position, Point(100, 100 + offset));
73-
EXPECT_POINT_NEAR(per_vertex[1].position, Point(200, 100 + offset));
74-
EXPECT_POINT_NEAR(per_vertex[2].position, Point(100, 100 - offset));
75-
EXPECT_POINT_NEAR(per_vertex[3].position, Point(200, 100 - offset));
72+
EXPECT_EQ(status.value().width, 5.f);
73+
EXPECT_EQ(status.value().radius, LineContents::kSampleRadius);
74+
EXPECT_POINT_NEAR(per_vertex[0].position,
75+
Point(100 - LineContents::kSampleRadius, 100 + offset));
76+
EXPECT_POINT_NEAR(per_vertex[1].position,
77+
Point(200 + LineContents::kSampleRadius, 100 + offset));
78+
EXPECT_POINT_NEAR(per_vertex[2].position,
79+
Point(100 - LineContents::kSampleRadius, 100 - offset));
80+
EXPECT_POINT_NEAR(per_vertex[3].position,
81+
Point(200 + LineContents::kSampleRadius, 100 - offset));
7682

7783
for (int i = 1; i < 4; ++i) {
7884
EXPECT_VECTOR3_NEAR(per_vertex[0].e0, per_vertex[i].e0) << i;
@@ -112,5 +118,45 @@ TEST(LineContents, CreateCurveDataScaled) {
112118
EXPECT_NEAR(data[3] / 255.f, 1.f, kEhCloseEnough);
113119
}
114120

121+
// This scales the line to be less than 1 pixel.
122+
TEST(LineContents, CalculatePerVertexLimit) {
123+
LineVertexShader::PerVertexData per_vertex[4];
124+
Scalar scale = 0.05;
125+
auto geometry = std::make_unique<LineGeometry>(
126+
/*p0=*/Point{100, 100}, //
127+
/*p1=*/Point{200, 100}, //
128+
/*width=*/10.f, //
129+
/*cap=*/Cap::kButt);
130+
Matrix transform = Matrix::MakeTranslation({100, 100, 1.0}) *
131+
Matrix::MakeScale({scale, scale, 1.0}) *
132+
Matrix::MakeTranslation({-100, -100, 1.0});
133+
134+
fml::StatusOr<LineContents::EffectiveLineParameters> status =
135+
LineContents::CalculatePerVertex(per_vertex, geometry.get(), transform);
136+
137+
Scalar one_radius_size = std::max(LineContents::kSampleRadius / scale,
138+
LineContents::kSampleRadius);
139+
Scalar one_px_size = 1.f / scale;
140+
Scalar offset = one_px_size / 2.f + one_radius_size;
141+
ASSERT_TRUE(status.ok());
142+
EXPECT_NEAR(status.value().width, 20.f, kEhCloseEnough);
143+
EXPECT_NEAR(status.value().radius, one_px_size * LineContents::kSampleRadius,
144+
kEhCloseEnough);
145+
EXPECT_POINT_NEAR(per_vertex[0].position,
146+
Point(100 - one_radius_size, 100 + offset));
147+
EXPECT_POINT_NEAR(per_vertex[1].position,
148+
Point(200 + one_radius_size, 100 + offset));
149+
EXPECT_POINT_NEAR(per_vertex[2].position,
150+
Point(100 - one_radius_size, 100 - offset));
151+
EXPECT_POINT_NEAR(per_vertex[3].position,
152+
Point(200 + one_radius_size, 100 - offset));
153+
154+
EXPECT_NEAR(CalculateLine(per_vertex[0], Point(150, 100)), 1.f,
155+
kEhCloseEnough);
156+
// EXPECT_NEAR(CalculateLine(per_vertex[0], Point(150, 100 +
157+
// one_px_size)), 1.f,
158+
// kEhCloseEnough);
159+
}
160+
115161
} // namespace testing
116162
} // namespace impeller

‎engine/src/flutter/impeller/entity/shaders/line.frag‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,10 @@ float CalculateLine() {
4242
void main() {
4343
float line = CalculateLine();
4444
frag_color = vec4(frag_info.color.xyz, line);
45+
//////////////////////////////////////////////////////////////////////////////
46+
// This is a nice way to visually debug this shader:
47+
// frag_color =
48+
// vec4(mix(vec3(1, 0,0), frag_info.color.xyz, line), 1.0);
49+
//////////////////////////////////////////////////////////////////////////////
4550
frag_color = IPPremultiply(frag_color);
4651
}

0 commit comments

Comments
 (0)