Skip to content

Commit 3919ddc

Browse files
adazem009adazem009
committed
Optimize color constants in set pen color block
1 parent 2f627e6 commit 3919ddc

File tree

5 files changed

+175
-30
lines changed

5 files changed

+175
-30
lines changed

‎src/blocks/penblocks.cpp‎

Lines changed: 79 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#include <scratchcpp/istagehandler.h>
77
#include <scratchcpp/value.h>
88
#include <scratchcpp/compilerconstant.h>
9+
#include <scratchcpp/stringptr.h>
10+
#include <scratchcpp/string_pool.h>
911

1012
#include "penblocks.h"
1113
#include "penlayer.h"
@@ -15,6 +17,35 @@
1517
using namespace scratchcpprender;
1618
using namespace libscratchcpp;
1719

20+
inline QColor pen_convert_from_numeric_color(long color)
21+
{
22+
return QColor::fromRgba(static_cast<QRgb>(color));
23+
}
24+
25+
static void pen_convert_color(const ValueData *color, QColor &dst)
26+
{
27+
StringPtr *str = string_pool_new();
28+
29+
if (value_isString(color)) {
30+
value_toStringPtr(color, str);
31+
32+
if (str->size > 0 && str->data[0] == u'#') {
33+
if (str->size <= 7) // #RRGGBB
34+
{
35+
dst = QColor::fromString(str->data);
36+
37+
if (!dst.isValid())
38+
dst = Qt::black;
39+
} else
40+
dst = Qt::black;
41+
} else
42+
dst = pen_convert_from_numeric_color(value_toLong(color));
43+
} else
44+
dst = pen_convert_from_numeric_color(value_toLong(color));
45+
46+
string_pool_free(str);
47+
}
48+
1849
std::string PenBlocks::name() const
1950
{
2051
return "pen";
@@ -42,7 +73,32 @@ void PenBlocks::registerBlocks(IEngine *engine)
4273
CompilerValue *PenBlocks::compileSetPenColorToColor(Compiler *compiler)
4374
{
4475
CompilerValue *color = compiler->addInput("COLOR");
45-
compiler->addTargetFunctionCall("pen_setPenColorToColor", Compiler::StaticType::Void, { Compiler::StaticType::Unknown }, { color });
76+
77+
if (color->isConst()) {
78+
// Convert color constant at compile time
79+
const ValueData *value = &dynamic_cast<CompilerConstant *>(color)->value().data();
80+
QColor converted;
81+
pen_convert_color(value, converted);
82+
83+
QColor hsv = converted.toHsv();
84+
CompilerValue *h = compiler->addConstValue((hsv.hue() / 360.0) * 100);
85+
CompilerValue *s = compiler->addConstValue(hsv.saturationF() * 100);
86+
CompilerValue *b = compiler->addConstValue(hsv.valueF() * 100);
87+
CompilerValue *transparency;
88+
89+
if (converted.alpha() > 0)
90+
transparency = compiler->addConstValue(100 * (1 - converted.alpha() / 255.0));
91+
else
92+
transparency = compiler->addConstValue(0);
93+
94+
compiler->addTargetFunctionCall(
95+
"pen_setPenColorToHsbColor",
96+
Compiler::StaticType::Void,
97+
{ Compiler::StaticType::Number, Compiler::StaticType::Number, Compiler::StaticType::Number, Compiler::StaticType::Number },
98+
{ h, s, b, transparency });
99+
} else
100+
compiler->addTargetFunctionCall("pen_setPenColorToColor", Compiler::StaticType::Void, { Compiler::StaticType::Unknown }, { color });
101+
46102
return nullptr;
47103
}
48104

@@ -120,44 +176,37 @@ BLOCK_EXPORT void pen_set_pen_down(Target *target, bool down)
120176
getTargetModel(target)->setPenDown(down);
121177
}
122178

123-
BLOCK_EXPORT void pen_setPenColorToColor(Target *target, const ValueData *color)
179+
BLOCK_EXPORT void pen_setPenColorToHsbColor(Target *target, double h, double s, double b, double transparency)
124180
{
125181
TargetModel *model = getTargetModel(target);
126182

127-
std::string stringValue;
128183
PenState &penState = model->penState();
129-
QColor newColor;
130-
131-
if (value_isString(color))
132-
value_toString(color, &stringValue);
184+
penState.color = h;
185+
penState.saturation = s;
186+
penState.brightness = b;
187+
penState.transparency = transparency;
133188

134-
if (!stringValue.empty() && stringValue[0] == '#') {
135-
bool valid = false;
136-
137-
if (stringValue.size() <= 7) // #RRGGBB
138-
{
139-
newColor = QColor::fromString(stringValue);
140-
valid = newColor.isValid();
141-
}
189+
penState.updateColor();
142190

143-
if (!valid)
144-
newColor = Qt::black;
191+
// Set the legacy "shade" value the same way Scratch 2 did.
192+
penState.shade = penState.brightness / 2;
193+
}
145194

146-
} else
147-
newColor = QColor::fromRgba(static_cast<QRgb>(value_toLong(color)));
195+
BLOCK_EXPORT void pen_setPenColorToColor(Target *target, const ValueData *color)
196+
{
197+
QColor converted;
198+
pen_convert_color(color, converted);
148199

149-
QColor hsv = newColor.toHsv();
150-
penState.color = (hsv.hue() / 360.0) * 100;
151-
penState.saturation = hsv.saturationF() * 100;
152-
penState.brightness = hsv.valueF() * 100;
200+
QColor hsv = converted.toHsv();
201+
double h = (hsv.hue() / 360.0) * 100;
202+
double s = hsv.saturationF() * 100;
203+
double b = hsv.valueF() * 100;
204+
double transparency;
153205

154-
if (newColor.alpha() > 0)
155-
penState.transparency = 100 * (1 - newColor.alpha() / 255.0);
206+
if (converted.alpha() > 0)
207+
transparency = 100 * (1 - converted.alpha() / 255.0);
156208
else
157-
penState.transparency = 0;
158-
159-
penState.updateColor();
209+
transparency = 0;
160210

161-
// Set the legacy "shade" value the same way Scratch 2 did.
162-
penState.shade = penState.brightness / 2;
211+
pen_setPenColorToHsbColor(target, h, s, b, transparency);
163212
}

‎test/blocks/CMakeLists.txt‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
add_library(
2+
block_test_deps SHARED
3+
util.cpp
4+
util.h
5+
)
6+
7+
target_link_libraries(
8+
block_test_deps
9+
GTest::gtest_main
10+
scratchcpp
11+
scratchcpp-render
12+
)
13+
114
# pen_blocks_test
215
add_executable(
316
pen_blocks_test
@@ -12,6 +25,7 @@ target_link_libraries(
1225
scratchcpp-render
1326
scratchcpprender_mocks
1427
qnanopainter
28+
block_test_deps
1529
${QT_LIBS}
1630
)
1731

‎test/blocks/pen_blocks_test.cpp‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,32 @@ TEST_F(PenBlocksTest, SetPenColorToColor_ValidHex)
243243
ASSERT_EQ(model.penAttributes().color, QNanoColor::fromQColor(QColor::fromHsv(359, 0, 0)));
244244
}
245245

246+
TEST_F(PenBlocksTest, SetPenColorToColor_NonConstHex)
247+
{
248+
auto sprite = std::make_shared<Sprite>();
249+
sprite->setEngine(&m_engineMock);
250+
251+
RenderedTarget renderedTarget;
252+
SpriteModel model;
253+
model.init(sprite.get());
254+
model.setRenderedTarget(&renderedTarget);
255+
sprite->setInterface(&model);
256+
257+
ScriptBuilder builder(m_extension.get(), m_engine, sprite);
258+
builder.addBlock("test_const_string");
259+
builder.addValueInput("STRING", "#FFGFFF");
260+
auto valueBlock = builder.takeBlock();
261+
262+
builder.addBlock("pen_setPenColorToColor");
263+
builder.addObscuredInput("COLOR", valueBlock);
264+
265+
auto thread = buildScript(builder, sprite.get());
266+
267+
EXPECT_CALL(m_engineMock, requestRedraw).Times(0);
268+
thread->run();
269+
ASSERT_EQ(model.penAttributes().color, QNanoColor::fromQColor(QColor::fromHsv(359, 0, 0)));
270+
}
271+
246272
TEST_F(PenBlocksTest, SetPenColorToColor_CaseInsensitiveHex)
247273
{
248274
auto sprite = std::make_shared<Sprite>();
@@ -360,6 +386,29 @@ TEST_F(PenBlocksTest, SetPenColorToColor_NumberWithoutAlphaChannel)
360386
ASSERT_EQ(model.penAttributes().color, QNanoColor::fromQColor(QColor::fromHsv(239, 255, 255)));
361387
}
362388

389+
TEST_F(PenBlocksTest, SetPenColorToColor_StringNumber)
390+
{
391+
auto sprite = std::make_shared<Sprite>();
392+
sprite->setEngine(&m_engineMock);
393+
394+
RenderedTarget renderedTarget;
395+
SpriteModel model;
396+
model.init(sprite.get());
397+
model.setRenderedTarget(&renderedTarget);
398+
sprite->setInterface(&model);
399+
400+
ScriptBuilder builder(m_extension.get(), m_engine, sprite);
401+
builder.addBlock("pen_setPenColorToColor");
402+
builder.addValueInput("COLOR", "255");
403+
404+
auto thread = buildScript(builder, sprite.get());
405+
406+
EXPECT_CALL(m_engineMock, requestRedraw).Times(0);
407+
thread->run();
408+
409+
ASSERT_EQ(model.penAttributes().color, QNanoColor::fromQColor(QColor::fromHsv(239, 255, 255)));
410+
}
411+
363412
TEST_F(PenBlocksTest, SetPenColorToColor_NumberWithAlphaChannel)
364413
{
365414
auto sprite = std::make_shared<Sprite>();

‎test/blocks/util.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include <scratchcpp/iengine.h>
2+
#include <scratchcpp/compiler.h>
3+
#include <scratchcpp/string_functions.h>
4+
5+
#include "util.h"
6+
7+
using namespace libscratchcpp;
8+
9+
static bool conditionReturnValue = false;
10+
11+
void registerBlocks(IEngine *engine, IExtension *extension)
12+
{
13+
engine->addCompileFunction(extension, "test_const_string", [](Compiler *compiler) -> CompilerValue * {
14+
auto input = compiler->addInput("STRING");
15+
return compiler->addFunctionCall("test_const_string", Compiler::StaticType::String, { Compiler::StaticType::String }, { input });
16+
});
17+
}
18+
19+
extern "C" void test_const_string(StringPtr *ret, const StringPtr *str)
20+
{
21+
string_assign(ret, str);
22+
}

‎test/blocks/util.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#pragma once
2+
3+
namespace libscratchcpp
4+
{
5+
6+
class IEngine;
7+
class IExtension;
8+
9+
} // namespace libscratchcpp
10+
11+
void registerBlocks(libscratchcpp::IEngine *engine, libscratchcpp::IExtension *extension);

0 commit comments

Comments
 (0)