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

Hooks up framework wide gamut to engine wide gamut #54567

Merged
auto-submit merged 5 commits into from
Aug 29, 2024
Merged

Hooks up framework wide gamut to engine wide gamut #54567

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
647a030
Hooks up framework wide gamut color to the engine.
gaaclarke Aug 15, 2024
67cd751
disabled that test for opengles
gaaclarke Aug 29, 2024
9a5403d
jonah feedback
gaaclarke Aug 29, 2024
555c462
jonah feedback
gaaclarke Aug 29, 2024
0a90ec7
brought back the colorspace conversion
gaaclarke Aug 29, 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
6 changes: 5 additions & 1 deletion impeller/display_list/aiks_dl_vertices_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "flutter/display_list/dl_color.h"
#include "flutter/display_list/dl_paint.h"
#include "flutter/testing/testing.h"
#include "impeller/display_list/dl_dispatcher.h"
#include "impeller/display_list/dl_image_impeller.h"

namespace impeller {
Expand Down Expand Up @@ -375,7 +376,10 @@ TEST_P(AiksTest, DrawVerticesWithInvalidIndices) {
builder.DrawRect(SkRect::MakeLTRB(0, 0, 400, 400), paint);
builder.DrawVertices(vertices, flutter::DlBlendMode::kSrc, paint);

ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
AiksContext renderer(GetContext(), nullptr);
std::shared_ptr<Texture> image =
DisplayListToTexture(builder.Build(), {1024, 768}, renderer);
EXPECT_TRUE(image);
}

// All four vertices should form a solid red rectangle with no gaps.
Expand Down
10 changes: 2 additions & 8 deletions lib/ui/painting/paint.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,8 @@ DlColor ReadColor(const tonic::DartByteData& byte_data) {
// Invert alpha so 0 initialized buffer has default value;
float alpha = 1.f - float_data[kColorAlphaIndex];
uint32_t colorspace = uint_data[kColorSpaceIndex];
(void)colorspace;
uint32_t encoded_color =
static_cast<uint8_t>(std::round(alpha * 255.f)) << 24 | //
static_cast<uint8_t>(std::round(red * 255.f)) << 16 | //
static_cast<uint8_t>(std::round(green * 255.f)) << 8 | //
static_cast<uint8_t>(std::round(blue * 255.f)) << 0;
// TODO(gaaclarke): Pass down color info to DlColor.
return DlColor(encoded_color);
return DlColor(alpha, red, green, blue,
Copy link
Contributor

@jonahwilliams jonahwilliams Aug 29, 2024

Choose a reason for hiding this comment

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

Shower thought, don't we need to do any color space conversions to RGBA/extend RGBA here? Or should we do that in dispatch based on the destination surface?

Copy link
Member Author

@gaaclarke gaaclarke Aug 29, 2024

Choose a reason for hiding this comment

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

The last PR added DlColor::withColorSpace for this conversion. The next PR is an integration test that makes sure everything works. It's been a couple of weeks since I tested the whole flow. It's possible it got lost in all this juggling. Let me double check I didn't lose it, because I'm not seeing a call to DlColor::withColorSpace anymore.

Copy link
Member Author

@gaaclarke gaaclarke Aug 29, 2024

Choose a reason for hiding this comment

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

You are correct, sir, reintroduced. When's that monorepo coming again?

static_cast<DlColorSpace>(colorspace));
}
} // namespace

Expand Down
3 changes: 0 additions & 3 deletions testing/impeller_golden_tests_output.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,9 +614,6 @@ impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShaderNonZeroOr
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Metal.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_OpenGLES.png
impeller_Play_AiksTest_DrawVerticesTextureCoordinatesWithFragmentShader_Vulkan.png
impeller_Play_AiksTest_DrawVerticesWithInvalidIndices_Metal.png
impeller_Play_AiksTest_DrawVerticesWithInvalidIndices_OpenGLES.png
impeller_Play_AiksTest_DrawVerticesWithInvalidIndices_Vulkan.png
impeller_Play_AiksTest_EmptySaveLayerIgnoresPaint_Metal.png
impeller_Play_AiksTest_EmptySaveLayerIgnoresPaint_OpenGLES.png
impeller_Play_AiksTest_EmptySaveLayerIgnoresPaint_Vulkan.png
Expand Down