Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Framework wide color linear gradients #54748

Merged
merged 7 commits into from
Sep 4, 2024
Merged

Framework wide color linear gradients #54748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
408f549
Framework wide gamut - implemented linear gradients
gaaclarke Aug 23, 2024
fc754cf
removed todo
gaaclarke Aug 23, 2024
d9c6a96
implemented radial gradient
gaaclarke Aug 23, 2024
976963c
added conical gradient
gaaclarke Aug 23, 2024
5c0887f
added sweep gradient
gaaclarke Aug 23, 2024
0419148
fixed color size
gaaclarke Sep 3, 2024
66b244e
jonah feedback 1, fixed overactive allocation
gaaclarke Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion impeller/display_list/skia_conversions.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ std::optional<impeller::PixelFormat> ToPixelFormat(SkColorType type) {
void ConvertStops(const flutter::DlGradientColorSourceBase* gradient,
std::vector<Color>& colors,
std::vector<float>& stops) {
FML_DCHECK(gradient->stop_count() >= 2);
FML_DCHECK(gradient->stop_count() >= 2)
<< "stop_count:" << gradient->stop_count();

auto* dl_colors = gradient->colors();
auto* dl_stops = gradient->stops();
Expand Down
35 changes: 25 additions & 10 deletions lib/ui/painting.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4452,10 +4452,25 @@ enum TileMode {
decal,
}

Float32List _encodeWideColorList(List<Color> colors) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember talking about preserving the CS, but I'm guessing that gradients are going to want to normalize to a similar CS for lerping - and extendedSRGB is probably the best for fractional combinations?

Copy link
Member Author

Choose a reason for hiding this comment

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

We do preserve the colorspace from the users perspective. The conversion to extended srgb only happens when communicating the values to the engine.

final int colorCount = colors.length;
final Float32List result = Float32List(colorCount * 4);
for (int i = 0; i < colorCount; i++) {
final Color colorXr =
colors[i].withValues(colorSpace: ColorSpace.extendedSRGB);
result[i*4+0] = colorXr.a;
result[i*4+1] = colorXr.r;
result[i*4+2] = colorXr.g;
result[i*4+3] = colorXr.b;
}
return result;
}


Int32List _encodeColorList(List<Color> colors) {
final int colorCount = colors.length;
final Int32List result = Int32List(colorCount);
for (int i = 0; i < colorCount; ++i) {
for (int i = 0; i < colorCount; i++) {
result[i] = colors[i].value;
}
return result;
Expand All @@ -4464,7 +4479,7 @@ Int32List _encodeColorList(List<Color> colors) {
Float32List _encodePointList(List<Offset> points) {
final int pointCount = points.length;
final Float32List result = Float32List(pointCount * 2);
for (int i = 0; i < pointCount; ++i) {
for (int i = 0; i < pointCount; i++) {
final int xIndex = i * 2;
final int yIndex = xIndex + 1;
final Offset point = points[i];
Expand Down Expand Up @@ -4535,7 +4550,7 @@ base class Gradient extends Shader {
super._() {
_validateColorStops(colors, colorStops);
final Float32List endPointsBuffer = _encodeTwoPoints(from, to);
final Int32List colorsBuffer = _encodeColorList(colors);
final Float32List colorsBuffer = _encodeWideColorList(colors);
final Float32List? colorStopsBuffer = colorStops == null ? null : Float32List.fromList(colorStops);
_constructor();
_initLinear(endPointsBuffer, colorsBuffer, colorStopsBuffer, tileMode.index, matrix4);
Expand Down Expand Up @@ -4588,8 +4603,8 @@ base class Gradient extends Shader {
assert(matrix4 == null || _matrix4IsValid(matrix4)),
super._() {
_validateColorStops(colors, colorStops);
final Int32List colorsBuffer = _encodeColorList(colors);
final Float32List? colorStopsBuffer = colorStops == null ? null : Float32List.fromList(colorStops);
final Float32List colorsBuffer = _encodeWideColorList(colors);

// If focal is null or focal radius is null, this should be treated as a regular radial gradient
// If focal == center and the focal radius is 0.0, it's still a regular radial gradient
Expand Down Expand Up @@ -4647,7 +4662,7 @@ base class Gradient extends Shader {
assert(matrix4 == null || _matrix4IsValid(matrix4)),
super._() {
_validateColorStops(colors, colorStops);
final Int32List colorsBuffer = _encodeColorList(colors);
final Float32List colorsBuffer = _encodeWideColorList(colors);
final Float32List? colorStopsBuffer = colorStops == null ? null : Float32List.fromList(colorStops);
_constructor();
_initSweep(center.dx, center.dy, colorsBuffer, colorStopsBuffer, tileMode.index, startAngle, endAngle, matrix4);
Expand All @@ -4657,14 +4672,14 @@ base class Gradient extends Shader {
external void _constructor();

@Native<Void Function(Pointer<Void>, Handle, Handle, Handle, Int32, Handle)>(symbol: 'Gradient::initLinear')
external void _initLinear(Float32List endPoints, Int32List colors, Float32List? colorStops, int tileMode, Float64List? matrix4);
external void _initLinear(Float32List endPoints, Float32List colors, Float32List? colorStops, int tileMode, Float64List? matrix4);

@Native<Void Function(Pointer<Void>, Double, Double, Double, Handle, Handle, Int32, Handle)>(symbol: 'Gradient::initRadial')
external void _initRadial(
double centerX,
double centerY,
double radius,
Int32List colors,
Float32List colors,
Float32List? colorStops,
int tileMode,
Float64List? matrix4);
Expand All @@ -4677,7 +4692,7 @@ base class Gradient extends Shader {
double endX,
double endY,
double endRadius,
Int32List colors,
Float32List colors,
Float32List? colorStops,
int tileMode,
Float64List? matrix4);
Expand All @@ -4686,7 +4701,7 @@ base class Gradient extends Shader {
external void _initSweep(
double centerX,
double centerY,
Int32List colors,
Float32List colors,
Float32List? colorStops,
int tileMode,
double startAngle,
Expand Down Expand Up @@ -6503,7 +6518,7 @@ base class _NativeCanvas extends NativeFieldWrapperClass1 implements Canvas {
final Float32List rstTransformBuffer = Float32List(rectCount * 4);
final Float32List rectBuffer = Float32List(rectCount * 4);

for (int i = 0; i < rectCount; ++i) {
for (int i = 0; i < rectCount; i++) {
final int index0 = i * 4;
final int index1 = index0 + 1;
final int index2 = index0 + 2;
Expand Down
79 changes: 49 additions & 30 deletions lib/ui/painting/gradient.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ void CanvasGradient::Create(Dart_Handle wrapper) {
}

void CanvasGradient::initLinear(const tonic::Float32List& end_points,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
const tonic::Float64List& matrix4) {
FML_DCHECK(end_points.num_elements() == 4);
FML_DCHECK(colors.num_elements() == color_stops.num_elements() ||
FML_DCHECK(colors.num_elements() == (color_stops.num_elements() * 4) ||
color_stops.data() == nullptr);
int num_colors = colors.num_elements() / 4;

static_assert(sizeof(SkPoint) == sizeof(float) * 2,
"SkPoint doesn't use floats.");
Expand All @@ -46,28 +47,32 @@ void CanvasGradient::initLinear(const tonic::Float32List& end_points,
SkPoint p0 = SkPoint::Make(end_points[0], end_points[1]);
SkPoint p1 = SkPoint::Make(end_points[2], end_points[3]);
std::vector<DlColor> dl_colors;
dl_colors.reserve(colors.num_elements());
for (int i = 0; i < colors.num_elements(); ++i) {
/// TODO(gaaclarke): Make this preserve wide gamut colors.
dl_colors.emplace_back(DlColor(colors[i]));
dl_colors.reserve(num_colors);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a helper method, or would that involve unnecessary copying? I thought the compiler optimized the case of returning a vector...??

Another way is to create the vector on the local stack and then hand it into the converter method by reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we have a helper method, or would that involve unnecessary copying? I thought the compiler optimized the case of returning a vector...??

Another way is to create the vector on the local stack and then hand it into the converter method by reference.

You can't have a vector on a stack. It will always allocate memory on the heap. The size is needed to be known at compilation time in order to put something on the stack (without trickery). I have a proposition in an above thread for using an std::array on the stack.

I think avoiding the copying will require DlColorSource some mechanism to read from the tonic collection directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

The data has to be heap allocated because it is growable, that will happen whether a helper function is used or not. I'm referring to the object itself, but I suppose that the object isn't very big whether it is stack allocated or not. It's a vtable + data pointer + allocated size + used size.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I see, sorry I thought this was still related to the heap discussion. Yes we could have a helper function and yes C++ would perform the return value optimization.

for (int i = 0; i < colors.num_elements(); i += 4) {
DlScalar a = colors[i + 0];
DlScalar r = colors[i + 1];
DlScalar g = colors[i + 2];
DlScalar b = colors[i + 3];
dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB));
}

dl_shader_ = DlColorSource::MakeLinear(
p0, p1, colors.num_elements(), dl_colors.data(), color_stops.data(),
tile_mode, has_matrix ? &sk_matrix : nullptr);
dl_shader_ = DlColorSource::MakeLinear(p0, p1, num_colors, dl_colors.data(),
color_stops.data(), tile_mode,
has_matrix ? &sk_matrix : nullptr);
// Just a sanity check, all gradient shaders should be thread-safe
FML_DCHECK(dl_shader_->isUIThreadSafe());
}

void CanvasGradient::initRadial(double center_x,
double center_y,
double radius,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
const tonic::Float64List& matrix4) {
FML_DCHECK(colors.num_elements() == color_stops.num_elements() ||
FML_DCHECK(colors.num_elements() == (color_stops.num_elements() * 4) ||
color_stops.data() == nullptr);
int num_colors = colors.num_elements() / 4;

static_assert(sizeof(SkColor) == sizeof(int32_t),
"SkColor doesn't use int32_t.");
Expand All @@ -79,29 +84,34 @@ void CanvasGradient::initRadial(double center_x,
}

std::vector<DlColor> dl_colors;
dl_colors.reserve(colors.num_elements());
for (int i = 0; i < colors.num_elements(); ++i) {
dl_colors.emplace_back(DlColor(colors[i]));
dl_colors.reserve(num_colors);
for (int i = 0; i < colors.num_elements(); i += 4) {
DlScalar a = colors[i + 0];
DlScalar r = colors[i + 1];
DlScalar g = colors[i + 2];
DlScalar b = colors[i + 3];
dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB));
}

dl_shader_ = DlColorSource::MakeRadial(
SkPoint::Make(SafeNarrow(center_x), SafeNarrow(center_y)),
SafeNarrow(radius), colors.num_elements(), dl_colors.data(),
color_stops.data(), tile_mode, has_matrix ? &sk_matrix : nullptr);
SafeNarrow(radius), num_colors, dl_colors.data(), color_stops.data(),
tile_mode, has_matrix ? &sk_matrix : nullptr);
// Just a sanity check, all gradient shaders should be thread-safe
FML_DCHECK(dl_shader_->isUIThreadSafe());
}

void CanvasGradient::initSweep(double center_x,
double center_y,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
double start_angle,
double end_angle,
const tonic::Float64List& matrix4) {
FML_DCHECK(colors.num_elements() == color_stops.num_elements() ||
FML_DCHECK(colors.num_elements() == (color_stops.num_elements() * 4) ||
color_stops.data() == nullptr);
int num_colors = colors.num_elements() / 4;

static_assert(sizeof(SkColor) == sizeof(int32_t),
"SkColor doesn't use int32_t.");
Expand All @@ -113,16 +123,20 @@ void CanvasGradient::initSweep(double center_x,
}

std::vector<DlColor> dl_colors;
dl_colors.reserve(colors.num_elements());
for (int i = 0; i < colors.num_elements(); ++i) {
dl_colors.emplace_back(DlColor(colors[i]));
dl_colors.reserve(num_colors);
for (int i = 0; i < colors.num_elements(); i += 4) {
DlScalar a = colors[i + 0];
DlScalar r = colors[i + 1];
DlScalar g = colors[i + 2];
DlScalar b = colors[i + 3];
dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB));
}

dl_shader_ = DlColorSource::MakeSweep(
SkPoint::Make(SafeNarrow(center_x), SafeNarrow(center_y)),
SafeNarrow(start_angle) * 180.0f / static_cast<float>(M_PI),
SafeNarrow(end_angle) * 180.0f / static_cast<float>(M_PI),
colors.num_elements(), dl_colors.data(), color_stops.data(), tile_mode,
SafeNarrow(end_angle) * 180.0f / static_cast<float>(M_PI), num_colors,
dl_colors.data(), color_stops.data(), tile_mode,
has_matrix ? &sk_matrix : nullptr);
// Just a sanity check, all gradient shaders should be thread-safe
FML_DCHECK(dl_shader_->isUIThreadSafe());
Expand All @@ -134,12 +148,13 @@ void CanvasGradient::initTwoPointConical(double start_x,
double end_x,
double end_y,
double end_radius,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
const tonic::Float64List& matrix4) {
FML_DCHECK(colors.num_elements() == color_stops.num_elements() ||
FML_DCHECK(colors.num_elements() == (color_stops.num_elements() * 4) ||
color_stops.data() == nullptr);
int num_colors = colors.num_elements() / 4;

static_assert(sizeof(SkColor) == sizeof(int32_t),
"SkColor doesn't use int32_t.");
Expand All @@ -151,17 +166,21 @@ void CanvasGradient::initTwoPointConical(double start_x,
}

std::vector<DlColor> dl_colors;
dl_colors.reserve(colors.num_elements());
for (int i = 0; i < colors.num_elements(); ++i) {
dl_colors.emplace_back(DlColor(colors[i]));
dl_colors.reserve(num_colors);
for (int i = 0; i < colors.num_elements(); i += 4) {
DlScalar a = colors[i + 0];
DlScalar r = colors[i + 1];
DlScalar g = colors[i + 2];
DlScalar b = colors[i + 3];
dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB));
}

dl_shader_ = DlColorSource::MakeConical(
SkPoint::Make(SafeNarrow(start_x), SafeNarrow(start_y)),
SafeNarrow(start_radius),
SkPoint::Make(SafeNarrow(end_x), SafeNarrow(end_y)),
SafeNarrow(end_radius), colors.num_elements(), dl_colors.data(),
color_stops.data(), tile_mode, has_matrix ? &sk_matrix : nullptr);
SafeNarrow(end_radius), num_colors, dl_colors.data(), color_stops.data(),
tile_mode, has_matrix ? &sk_matrix : nullptr);
// Just a sanity check, all gradient shaders should be thread-safe
FML_DCHECK(dl_shader_->isUIThreadSafe());
}
Expand Down
8 changes: 4 additions & 4 deletions lib/ui/painting/gradient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,22 @@ class CanvasGradient : public Shader {
static void Create(Dart_Handle wrapper);

void initLinear(const tonic::Float32List& end_points,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
const tonic::Float64List& matrix4);

void initRadial(double center_x,
double center_y,
double radius,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
const tonic::Float64List& matrix4);

void initSweep(double center_x,
double center_y,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
double start_angle,
Expand All @@ -49,7 +49,7 @@ class CanvasGradient : public Shader {
double end_x,
double end_y,
double end_radius,
const tonic::Int32List& colors,
const tonic::Float32List& colors,
const tonic::Float32List& color_stops,
DlTileMode tile_mode,
const tonic::Float64List& matrix4);
Expand Down