Skip to content

Commit 8d5a64d

Browse files
committed
Rewrite pen color logic
1 parent 5a5723d commit 8d5a64d

File tree

5 files changed

+71
-80
lines changed

5 files changed

+71
-80
lines changed

src/blocks/penblocks.cpp

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,10 @@ unsigned int PenBlocks::changePenHueBy(libscratchcpp::VirtualMachine *vm)
140140
SpriteModel *model = getSpriteModel(vm);
141141

142142
if (model) {
143+
PenState &penState = model->penState();
143144
const double colorChange = vm->getInput(0, 1)->toDouble() / 2;
144-
setOrChangeColorParam(ColorParam::COLOR, colorChange, model->penState(), true, true);
145+
setOrChangeColorParam(ColorParam::COLOR, colorChange, penState, true);
146+
legacyUpdatePenColor(penState);
145147
}
146148

147149
return 1;
@@ -152,10 +154,11 @@ unsigned int PenBlocks::setPenHueToNumber(libscratchcpp::VirtualMachine *vm)
152154
SpriteModel *model = getSpriteModel(vm);
153155

154156
if (model) {
157+
PenState &penState = model->penState();
155158
const double colorValue = vm->getInput(0, 1)->toDouble() / 2;
156-
setOrChangeColorParam(ColorParam::COLOR, colorValue, model->penState(), false, true);
157-
model->penState().transparency = 0;
158-
model->penState().updateColor();
159+
setOrChangeColorParam(ColorParam::COLOR, colorValue, penState, false);
160+
penState.transparency = 0;
161+
legacyUpdatePenColor(penState);
159162
}
160163

161164
return 1;
@@ -186,14 +189,20 @@ unsigned int PenBlocks::setPenColorToColor(libscratchcpp::VirtualMachine *vm)
186189
if (!valid)
187190
newColor = Qt::black;
188191

189-
} else {
192+
} else
190193
newColor = QColor::fromRgba(static_cast<QRgb>(value->toLong()));
191194

192-
if (newColor.alpha() == 0)
193-
newColor.setAlpha(255);
194-
}
195+
QColor hsv = newColor.toHsv();
196+
penState.color = (hsv.hue() / 360.0) * 100;
197+
penState.saturation = hsv.saturationF() * 100;
198+
penState.brightness = hsv.valueF() * 100;
199+
200+
if (newColor.alpha() > 0)
201+
penState.transparency = 100 * (1 - newColor.alpha() / 255.0);
202+
else
203+
penState.transparency = 0;
195204

196-
penState.setColor(newColor);
205+
penState.updateColor();
197206

198207
// Set the legacy "shade" value the same way Scratch 2 did.
199208
penState.shade = penState.brightness / 2;
@@ -214,7 +223,7 @@ SpriteModel *PenBlocks::getSpriteModel(libscratchcpp::VirtualMachine *vm)
214223
return model;
215224
}
216225

217-
void PenBlocks::setOrChangeColorParam(ColorParam param, double value, PenState &penState, bool change, bool legacy)
226+
void PenBlocks::setOrChangeColorParam(ColorParam param, double value, PenState &penState, bool change)
218227
{
219228
switch (param) {
220229
case ColorParam::COLOR:
@@ -223,39 +232,49 @@ void PenBlocks::setOrChangeColorParam(ColorParam param, double value, PenState &
223232
}
224233

225234
penState.updateColor();
235+
}
226236

227-
if (legacy) {
228-
// https://github.com/scratchfoundation/scratch-vm/blob/8dbcc1fc8f8d8c4f1e40629fe8a388149d6dfd1c/src/extensions/scratch3_pen/index.js#L750-L767
229-
// Create the new color in RGB using the scratch 2 "shade" model
230-
QColor rgb = penState.penAttributes.color.toRgb();
231-
const double shade = (penState.shade > 100) ? 200 - penState.shade : penState.shade;
232-
233-
if (shade < 50)
234-
rgb = mixRgb(QColor(Qt::black), rgb, (10 + shade) / 60);
235-
else
236-
rgb = mixRgb(rgb, QColor(Qt::white), (shade - 50) / 60);
237+
void PenBlocks::legacyUpdatePenColor(PenState &penState)
238+
{
239+
// https://github.com/scratchfoundation/scratch-vm/blob/8dbcc1fc8f8d8c4f1e40629fe8a388149d6dfd1c/src/extensions/scratch3_pen/index.js#L750-L767
240+
// Create the new color in RGB using the scratch 2 "shade" model
241+
QRgb rgb = QColor::fromHsvF(penState.color / 100, 1, 1).rgb();
242+
const double shade = (penState.shade > 100) ? 200 - penState.shade : penState.shade;
243+
244+
if (shade < 50)
245+
rgb = mixRgb(0, rgb, (10 + shade) / 60);
246+
else
247+
rgb = mixRgb(rgb, 0xFFFFFF, (shade - 50) / 60);
248+
249+
// Update the pen state according to new color
250+
QColor hsv = QColor::fromRgb(rgb).toHsv();
251+
penState.color = 100 * hsv.hueF();
252+
penState.saturation = 100 * hsv.saturationF();
253+
penState.brightness = 100 * hsv.valueF();
237254

238-
// Update the pen state according to new color
239-
const QColor hsv = rgb.toHsv();
240-
penState.setColor(hsv);
241-
}
255+
penState.updateColor();
242256
}
243257

244258
double PenBlocks::wrapClamp(double n, double min, double max)
245259
{
246260
// TODO: Move this to a separate class
247-
const double range = max - min + 1;
261+
const double range = max - min /*+ 1*/;
248262
return n - (std::floor((n - min) / range) * range);
249263
}
250264

251-
QColor PenBlocks::mixRgb(const QColor &rgb0, const QColor &rgb1, double fraction1)
265+
QRgb PenBlocks::mixRgb(QRgb rgb0, QRgb rgb1, double fraction1)
252266
{
253267
// https://github.com/scratchfoundation/scratch-vm/blob/a4f095db5e03e072ba222fe721eeeb543c9b9c15/src/util/color.js#L192-L201
268+
// https://github.com/scratchfoundation/scratch-flash/blob/2e4a402ceb205a042887f54b26eebe1c2e6da6c0/src/util/Color.as#L75-L89
254269
if (fraction1 <= 0)
255270
return rgb0;
271+
256272
if (fraction1 >= 1)
257273
return rgb1;
258274

259275
const double fraction0 = 1 - fraction1;
260-
return QColor((fraction0 * rgb0.red()) + (fraction1 * rgb1.red()), (fraction0 * rgb0.green()) + (fraction1 * rgb1.green()), (fraction0 * rgb0.blue()) + (fraction1 * rgb1.blue()));
276+
const int r = static_cast<int>(((fraction0 * qRed(rgb0)) + (fraction1 * qRed(rgb1)))) & 255;
277+
const int g = static_cast<int>(((fraction0 * qGreen(rgb0)) + (fraction1 * qGreen(rgb1)))) & 255;
278+
const int b = static_cast<int>(((fraction0 * qBlue(rgb0)) + (fraction1 * qBlue(rgb1)))) & 255;
279+
return qRgb(r, g, b);
261280
}

src/blocks/penblocks.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22

33
#pragma once
44

5+
#include <QColor>
56
#include <scratchcpp/iblocksection.h>
67

7-
class QColor;
8-
98
namespace scratchcpprender
109
{
1110

@@ -51,9 +50,10 @@ class PenBlocks : public libscratchcpp::IBlockSection
5150
};
5251

5352
static SpriteModel *getSpriteModel(libscratchcpp::VirtualMachine *vm);
54-
static void setOrChangeColorParam(ColorParam param, double value, PenState &penState, bool change, bool legacy = false);
53+
static void setOrChangeColorParam(ColorParam param, double value, PenState &penState, bool change);
54+
static void legacyUpdatePenColor(PenState &penState);
5555
static double wrapClamp(double n, double min, double max);
56-
static QColor mixRgb(const QColor &rgb0, const QColor &rgb1, double fraction1);
56+
static QRgb mixRgb(QRgb rgb0, QRgb rgb1, double fraction1);
5757
};
5858

5959
} // namespace scratchcpprender

src/penstate.h

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,28 +19,17 @@ struct PenState
1919
double shade = 50; // for legacy blocks
2020
PenAttributes penAttributes;
2121

22-
void setColor(const QColor &color)
23-
{
24-
QColor hsvColor = color.toHsv();
25-
this->color = hsvColor.hue() * 100 / 360.0;
26-
this->saturation = hsvColor.saturationF() * 100;
27-
this->brightness = hsvColor.valueF() * 100;
28-
this->transparency = 100 * (1 - hsvColor.alphaF());
29-
30-
penAttributes.color = color;
31-
}
32-
3322
void updateColor()
3423
{
35-
int h = std::round(color * 360 / 100);
24+
int h = color * 360 / 100;
3625
h %= 360;
3726

3827
if (h < 0)
3928
h += 360;
4029

41-
const int s = std::round(saturation * 2.55);
42-
const int v = std::round(brightness * 2.55);
43-
const int a = std::round((100 - transparency) * 2.55);
30+
const int s = saturation * 255 / 100;
31+
const int v = brightness * 255 / 100;
32+
const int a = 255 - transparency * 255 / 100;
4433

4534
penAttributes.color = QColor::fromHsv(h, s, v, a);
4635
}

test/blocks/pen_blocks_test.cpp

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -252,43 +252,43 @@ TEST_F(PenBlocksTest, SetPenColorToColorImpl)
252252

253253
vm.run();
254254
ASSERT_EQ(vm.registerCount(), 0);
255-
ASSERT_EQ(model.penAttributes().color, QColor(170, 187, 204));
255+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(210, 42, 204));
256256

257257
vm.reset();
258258
vm.setBytecode(bytecode2);
259259
vm.run();
260260
ASSERT_EQ(vm.registerCount(), 0);
261-
ASSERT_EQ(model.penAttributes().color, QColor(0, 51, 255));
261+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(228, 255, 255));
262262

263263
vm.reset();
264264
vm.setBytecode(bytecode3);
265265
vm.run();
266266
ASSERT_EQ(vm.registerCount(), 0);
267-
ASSERT_EQ(model.penAttributes().color, QColor(0, 0, 0));
267+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(359, 0, 0));
268268

269269
vm.reset();
270270
vm.setBytecode(bytecode4);
271271
vm.run();
272272
ASSERT_EQ(vm.registerCount(), 0);
273-
ASSERT_EQ(model.penAttributes().color, QColor(0, 0, 0));
273+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(359, 0, 0));
274274

275275
vm.reset();
276276
vm.setBytecode(bytecode5);
277277
vm.run();
278278
ASSERT_EQ(vm.registerCount(), 0);
279-
ASSERT_EQ(model.penAttributes().color, QColor(0, 0, 0));
279+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(359, 0, 0));
280280

281281
vm.reset();
282282
vm.setBytecode(bytecode6);
283283
vm.run();
284284
ASSERT_EQ(vm.registerCount(), 0);
285-
ASSERT_EQ(model.penAttributes().color, QColor::fromRgba(1228097602));
285+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(162, 74, 72, 73));
286286

287287
vm.reset();
288288
vm.setBytecode(bytecode7);
289289
vm.run();
290290
ASSERT_EQ(vm.registerCount(), 0);
291-
ASSERT_EQ(model.penAttributes().color, QColor::fromRgb(255));
291+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(239, 255, 255));
292292
}
293293

294294
TEST_F(PenBlocksTest, ChangePenSizeBy)
@@ -468,9 +468,7 @@ TEST_F(PenBlocksTest, ChangePenHueByImpl)
468468
static Value constValues[] = { 125.7, -114.09 };
469469

470470
SpriteModel model;
471-
QColor color = model.penAttributes().color;
472-
color.setAlpha(150);
473-
model.penState().setColor(color);
471+
model.penState().transparency = 100 * (1 - 150 / 255.0);
474472
Sprite sprite;
475473
sprite.setInterface(&model);
476474

@@ -481,28 +479,28 @@ TEST_F(PenBlocksTest, ChangePenHueByImpl)
481479

482480
vm.run();
483481
ASSERT_EQ(vm.registerCount(), 0);
484-
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(103, 255, 255, 150));
482+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(106, 255, 255, 150));
485483

486484
vm.reset();
487485
vm.run();
488486
ASSERT_EQ(vm.registerCount(), 0);
489-
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(329, 255, 255, 150));
487+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(332, 255, 255, 150));
490488

491489
vm.reset();
492490
vm.run();
493491
ASSERT_EQ(vm.registerCount(), 0);
494-
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(192, 255, 255, 150));
492+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(199, 255, 255, 150));
495493

496494
vm.reset();
497495
vm.setBytecode(bytecode2);
498496
vm.run();
499497
ASSERT_EQ(vm.registerCount(), 0);
500-
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(350, 255, 255, 150));
498+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(353, 255, 255, 150));
501499

502500
vm.reset();
503501
vm.run();
504502
ASSERT_EQ(vm.registerCount(), 0);
505-
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(145, 255, 255, 150));
503+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(148, 255, 255, 150));
506504
}
507505

508506
TEST_F(PenBlocksTest, SetPenHueToNumber)
@@ -545,9 +543,7 @@ TEST_F(PenBlocksTest, SetPenHueToNumberImpl)
545543
static Value constValues[] = { 125.7, -114.09, 489.4 };
546544

547545
SpriteModel model;
548-
QColor color = model.penAttributes().color;
549-
color.setAlpha(150);
550-
model.penState().setColor(color);
546+
model.penState().transparency = 100 * (1 - 150 / 255.0);
551547
Sprite sprite;
552548
sprite.setInterface(&model);
553549

@@ -564,11 +560,11 @@ TEST_F(PenBlocksTest, SetPenHueToNumberImpl)
564560
vm.setBytecode(bytecode2);
565561
vm.run();
566562
ASSERT_EQ(vm.registerCount(), 0);
567-
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(158, 255, 255, 255));
563+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(154, 255, 255, 255));
568564

569565
vm.reset();
570566
vm.setBytecode(bytecode3);
571567
vm.run();
572568
ASSERT_EQ(vm.registerCount(), 0);
573-
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(154, 255, 255, 255));
569+
ASSERT_EQ(model.penAttributes().color, QColor::fromHsv(160, 255, 255, 255));
574570
}

test/penstate/penstate_test.cpp

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,6 @@ TEST(PenStateTest, DefaultPenState)
1919
ASSERT_EQ(state.penAttributes.diameter, defaultAttributes.diameter);
2020
}
2121

22-
TEST(PenStateTest, SetColor)
23-
{
24-
PenState state;
25-
QColor color(64, 20, 189, 167);
26-
state.setColor(color);
27-
ASSERT_FALSE(state.penDown);
28-
ASSERT_EQ(std::round(state.color * 100) / 100, 70.83);
29-
ASSERT_EQ(std::round(state.saturation * 100) / 100, 89.42);
30-
ASSERT_EQ(std::round(state.brightness * 100) / 100, 74.12);
31-
ASSERT_EQ(std::round(state.transparency * 100) / 100, 34.51);
32-
ASSERT_EQ(state.shade, 50);
33-
}
34-
3522
TEST(PenStateTest, UpdateColor)
3623
{
3724
PenState state;
@@ -41,5 +28,5 @@ TEST(PenStateTest, UpdateColor)
4128
state.transparency = 36.09;
4229
state.shade = 85;
4330
state.updateColor();
44-
ASSERT_EQ(state.penAttributes.color, QColor::fromHsv(283, 115, 32, 163));
31+
ASSERT_EQ(state.penAttributes.color, QColor::fromHsv(283, 114, 31, 162));
4532
}

0 commit comments

Comments
 (0)