Skip to content

Commit b97f774

Browse files
wcandillonwcandillon
authored
fix(🎨): fix opacity regression with layers (#3185)
1 parent 7ab9c66 commit b97f774

File tree

14 files changed

+114
-71
lines changed

14 files changed

+114
-71
lines changed

‎packages/skia/cpp/api/recorder/DrawingCtx.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,17 @@ class DrawingCtx {
6060
}
6161

6262
float getOpacity() const { return opacities.back(); }
63-
63+
6464
void setOpacity(float newOpacity) {
6565
opacities.back() = std::clamp(newOpacity, 0.0f, 1.0f);
6666
}
6767

68-
void pushPaint(SkPaint &paint) {
68+
void pushPaint(SkPaint &paint) {
6969
paints.push_back(paint);
7070
opacities.push_back(opacities.back());
7171
}
7272

73-
void savePaint() {
73+
void savePaint() {
7474
paints.push_back(SkPaint(getPaint()));
7575
opacities.push_back(opacities.back());
7676
}

‎packages/skia/cpp/api/recorder/JsiRecorder.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ class JsiRecorder : public JsiSkWrappingSharedPtrHostObject<Recorder> {
2525
std::make_shared<Recorder>()) {}
2626

2727
JSI_HOST_FUNCTION(savePaint) {
28-
getObject()->savePaint(runtime, arguments[0].asObject(runtime));
28+
getObject()->savePaint(runtime, arguments[0].asObject(runtime),
29+
arguments[1].asBool());
2930
return jsi::Value::undefined();
3031
}
3132

‎packages/skia/cpp/api/recorder/Paint.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,12 @@ struct PaintCmdProps {
125125
class SavePaintCmd : public Command {
126126
private:
127127
PaintCmdProps props;
128+
bool standalone;
128129

129130
public:
130131
SavePaintCmd(jsi::Runtime &runtime, const jsi::Object &object,
131-
Variables &variables)
132-
: Command(CommandType::SavePaint) {
132+
Variables &variables, bool lStandalone)
133+
: Command(CommandType::SavePaint), standalone(lStandalone) {
133134
convertProperty(runtime, object, "color", props.color, variables);
134135
convertProperty(runtime, object, "blendMode", props.blendMode, variables);
135136
convertProperty(runtime, object, "style", props.style, variables);
@@ -153,7 +154,11 @@ class SavePaintCmd : public Command {
153154
ctx->savePaint();
154155
auto &paint = ctx->getPaint();
155156
if (props.opacity.has_value()) {
156-
ctx->setOpacity(ctx->getOpacity() * props.opacity.value());
157+
if (standalone) {
158+
paint.setAlphaf(paint.getAlphaf() * props.opacity.value());
159+
} else {
160+
ctx->setOpacity(ctx->getOpacity() * props.opacity.value());
161+
}
157162
}
158163
if (props.color.has_value()) {
159164
paint.setShader(nullptr);

‎packages/skia/cpp/api/recorder/RNRecorder.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ class Recorder {
2828

2929
~Recorder() = default;
3030

31-
void savePaint(jsi::Runtime &runtime, const jsi::Object &props) {
31+
void savePaint(jsi::Runtime &runtime, const jsi::Object &props,
32+
bool standalone) {
3233
commands.push_back(
33-
std::make_unique<SavePaintCmd>(runtime, props, variables));
34+
std::make_unique<SavePaintCmd>(runtime, props, variables, standalone));
3435
}
3536

3637
void pushShader(jsi::Runtime &runtime, const std::string &nodeType,
@@ -294,7 +295,7 @@ class Recorder {
294295
void play(DrawingCtx *ctx) {
295296
for (const auto &cmd : commands) {
296297
switch (cmd->type) {
297-
298+
298299
case Group: {
299300
// Do nothing here for now
300301
break;

‎packages/skia/cpp/rnskia/RNSkJsiViewApi.h

Lines changed: 53 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ class ViewRegistry {
4040
}
4141

4242
// Execute a function while holding the registry lock
43-
template<typename F>
44-
auto withViewInfo(size_t id, F&& func) -> decltype(func(std::shared_ptr<RNSkViewInfo>())) {
43+
template <typename F>
44+
auto withViewInfo(size_t id, F &&func)
45+
-> decltype(func(std::shared_ptr<RNSkViewInfo>())) {
4546
std::unique_lock<std::shared_mutex> lock(_mutex);
4647
auto it = _registry.find(id);
4748
std::shared_ptr<RNSkViewInfo> info;
@@ -95,21 +96,23 @@ class RNSkJsiViewApi : public RNJsi::JsiHostObject,
9596
}
9697

9798
auto nativeId = arguments[0].asNumber();
98-
99+
99100
// Safely execute operations while holding the registry lock
100-
ViewRegistry::getInstance().withViewInfo(nativeId, [&](std::shared_ptr<RNSkViewInfo> info) {
101-
info->props.insert_or_assign(arguments[1].asString(runtime).utf8(runtime),
102-
RNJsi::ViewProperty(runtime, arguments[2]));
103-
104-
// Now let's see if we have a view that we can update
105-
if (info->view != nullptr) {
106-
// Update view!
107-
info->view->setNativeId(nativeId);
108-
info->view->setJsiProperties(info->props);
109-
info->props.clear();
110-
}
111-
return nullptr; // Return type for template deduction
112-
});
101+
ViewRegistry::getInstance().withViewInfo(
102+
nativeId, [&](std::shared_ptr<RNSkViewInfo> info) {
103+
info->props.insert_or_assign(
104+
arguments[1].asString(runtime).utf8(runtime),
105+
RNJsi::ViewProperty(runtime, arguments[2]));
106+
107+
// Now let's see if we have a view that we can update
108+
if (info->view != nullptr) {
109+
// Update view!
110+
info->view->setNativeId(nativeId);
111+
info->view->setJsiProperties(info->props);
112+
info->props.clear();
113+
}
114+
return nullptr; // Return type for template deduction
115+
});
113116

114117
return jsi::Value::undefined();
115118
}
@@ -132,12 +135,13 @@ class RNSkJsiViewApi : public RNJsi::JsiHostObject,
132135

133136
// find Skia View
134137
int nativeId = arguments[0].asNumber();
135-
ViewRegistry::getInstance().withViewInfo(nativeId, [](std::shared_ptr<RNSkViewInfo> info) {
136-
if (info->view != nullptr) {
137-
info->view->requestRedraw();
138-
}
139-
return nullptr;
140-
});
138+
ViewRegistry::getInstance().withViewInfo(
139+
nativeId, [](std::shared_ptr<RNSkViewInfo> info) {
140+
if (info->view != nullptr) {
141+
info->view->requestRedraw();
142+
}
143+
return nullptr;
144+
});
141145
return jsi::Value::undefined();
142146
}
143147

@@ -158,10 +162,9 @@ class RNSkJsiViewApi : public RNJsi::JsiHostObject,
158162
// find Skia view
159163
int nativeId = arguments[0].asNumber();
160164
sk_sp<SkImage> image;
161-
std::shared_ptr<RNSkView> view = ViewRegistry::getInstance().withViewInfo(nativeId,
162-
[](std::shared_ptr<RNSkViewInfo> info) {
163-
return info->view;
164-
});
165+
std::shared_ptr<RNSkView> view = ViewRegistry::getInstance().withViewInfo(
166+
nativeId,
167+
[](std::shared_ptr<RNSkViewInfo> info) { return info->view; });
165168
if (view != nullptr) {
166169
if (count > 1 && !arguments[1].isUndefined() && !arguments[1].isNull()) {
167170
auto rect = JsiSkRect::fromValue(runtime, arguments[1]);
@@ -197,10 +200,9 @@ class RNSkJsiViewApi : public RNJsi::JsiHostObject,
197200

198201
// find Skia view
199202
int nativeId = arguments[0].asNumber();
200-
std::shared_ptr<RNSkView> view = ViewRegistry::getInstance().withViewInfo(nativeId,
201-
[](std::shared_ptr<RNSkViewInfo> info) {
202-
return info->view;
203-
});
203+
std::shared_ptr<RNSkView> view = ViewRegistry::getInstance().withViewInfo(
204+
nativeId,
205+
[](std::shared_ptr<RNSkViewInfo> info) { return info->view; });
204206
auto context = _platformContext;
205207
auto bounds =
206208
count > 1 && !arguments[1].isUndefined() && !arguments[1].isNull()
@@ -245,23 +247,22 @@ class RNSkJsiViewApi : public RNJsi::JsiHostObject,
245247
/**
246248
Call to remove all draw view infos
247249
*/
248-
void unregisterAll() {
249-
ViewRegistry::getInstance().clear();
250-
}
250+
void unregisterAll() { ViewRegistry::getInstance().clear(); }
251251

252252
/**
253253
* Registers a skia view
254254
* @param nativeId Id of view to register
255255
* @param view View to register
256256
*/
257257
void registerSkiaView(size_t nativeId, std::shared_ptr<RNSkView> view) {
258-
ViewRegistry::getInstance().withViewInfo(nativeId, [&](std::shared_ptr<RNSkViewInfo> info) {
259-
info->view = view;
260-
info->view->setNativeId(nativeId);
261-
info->view->setJsiProperties(info->props);
262-
info->props.clear();
263-
return nullptr;
264-
});
258+
ViewRegistry::getInstance().withViewInfo(
259+
nativeId, [&](std::shared_ptr<RNSkViewInfo> info) {
260+
info->view = view;
261+
info->view->setNativeId(nativeId);
262+
info->view->setJsiProperties(info->props);
263+
info->props.clear();
264+
return nullptr;
265+
});
265266
}
266267

267268
/**
@@ -279,17 +280,18 @@ class RNSkJsiViewApi : public RNJsi::JsiHostObject,
279280
or a valid view, effectively toggling the view's availability.
280281
*/
281282
void setSkiaView(size_t nativeId, std::shared_ptr<RNSkView> view) {
282-
ViewRegistry::getInstance().withViewInfo(nativeId, [&](std::shared_ptr<RNSkViewInfo> info) {
283-
if (view != nullptr) {
284-
info->view = view;
285-
info->view->setNativeId(nativeId);
286-
info->view->setJsiProperties(info->props);
287-
info->props.clear();
288-
} else {
289-
info->view = view; // Set to nullptr
290-
}
291-
return nullptr;
292-
});
283+
ViewRegistry::getInstance().withViewInfo(
284+
nativeId, [&](std::shared_ptr<RNSkViewInfo> info) {
285+
if (view != nullptr) {
286+
info->view = view;
287+
info->view->setNativeId(nativeId);
288+
info->view->setJsiProperties(info->props);
289+
info->props.clear();
290+
} else {
291+
info->view = view; // Set to nullptr
292+
}
293+
return nullptr;
294+
});
293295
}
294296

295297
private:

‎packages/skia/src/__tests__/snapshots/drawings/semi-transparent-layer.png

3.96 KB
Loading

‎packages/skia/src/__tests__/snapshots/drawings/transparent-layer.png

2.34 KB
Loading

‎packages/skia/src/renderer/__tests__/e2e/Opacity.spec.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
Group,
1515
Image,
1616
ImageShader,
17+
Paint,
1718
RoundedRect,
1819
} from "../../components";
1920
import { setupSkia } from "../../../skia/__tests__/setup";
@@ -275,4 +276,26 @@ describe("Opacity", () => {
275276
);
276277
checkImage(img, "snapshots/drawings/opacity-image.png");
277278
});
279+
it("Should apply opacity on a layer (1)", async () => {
280+
const {} = importSkia();
281+
const img = await surface.draw(
282+
<Group>
283+
<Group layer={<Paint opacity={0.5} />}>
284+
<Fill color="lightblue" />
285+
</Group>
286+
</Group>
287+
);
288+
checkImage(img, "snapshots/drawings/semi-transparent-layer.png");
289+
});
290+
it("Should apply opacity on a layer (2)", async () => {
291+
const {} = importSkia();
292+
const img = await surface.draw(
293+
<Group>
294+
<Group layer={<Paint opacity={0} />}>
295+
<Fill color="lightblue" />
296+
</Group>
297+
</Group>
298+
);
299+
checkImage(img, "snapshots/drawings/transparent-layer.png");
300+
});
278301
});

‎packages/skia/src/skia/types/Recorder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import type { SkPicture } from "./Picture";
3434
export interface BaseRecorder {
3535
saveGroup(): void;
3636
restoreGroup(): void;
37-
savePaint(props: AnimatedProps<PaintProps>): void;
37+
savePaint(props: AnimatedProps<PaintProps>, standalone: boolean): void;
3838
restorePaint(): void;
3939
restorePaintDeclaration(): void;
4040
materializePaint(): void;

‎packages/skia/src/sksg/Recorder/Player.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,13 @@ function play(ctx: DrawingContext, _command: Command) {
6767
ctx.paints.push(command.props.paint);
6868
} else {
6969
ctx.savePaint();
70-
setPaintProperties(ctx.Skia, ctx, command.props);
70+
setPaintProperties(
71+
ctx.Skia,
72+
ctx,
73+
command.props,
74+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
75+
(command as any).standalone
76+
);
7177
}
7278
} else if (isCommand(command, CommandType.RestorePaint)) {
7379
ctx.restorePaint();

0 commit comments

Comments
 (0)