Skip to content

Commit 77b4d1c

Browse files
adazem009adazem009
committed
Refactor pen frame state management
1 parent 887823d commit 77b4d1c

File tree

7 files changed

+117
-33
lines changed

7 files changed

+117
-33
lines changed

‎src/ipenlayer.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ class IPenLayer : public QNanoQuickItem
3434
virtual libscratchcpp::IEngine *engine() const = 0;
3535
virtual void setEngine(libscratchcpp::IEngine *newEngine) = 0;
3636

37+
virtual void beginFrame() = 0;
38+
virtual void endFrame() = 0;
39+
3740
virtual void clear() = 0;
3841
virtual void drawPoint(const PenAttributes &penAttributes, double x, double y) = 0;
3942
virtual void drawLine(const PenAttributes &penAttributes, double x0, double y0, double x1, double y1) = 0;

‎src/penlayer.cpp‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ void PenLayer::setEngine(libscratchcpp::IEngine *newEngine)
121121
emit engineChanged();
122122
}
123123

124+
void PenLayer::beginFrame()
125+
{
126+
m_fbo->bind();
127+
m_painter->beginFrame(m_fbo->width(), m_fbo->height());
128+
}
129+
130+
void PenLayer::endFrame()
131+
{
132+
m_painter->endFrame();
133+
m_fbo->release();
134+
}
135+
124136
bool PenLayer::hqPen() const
125137
{
126138
return m_hqPen;
@@ -141,12 +153,11 @@ void scratchcpprender::PenLayer::clear()
141153
if (!m_fbo)
142154
return;
143155

144-
m_fbo->bind();
156+
Q_ASSERT(m_fbo->isBound());
145157
m_glF->glDisable(GL_SCISSOR_TEST);
146158
m_glF->glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
147159
m_glF->glClear(GL_COLOR_BUFFER_BIT);
148160
m_glF->glEnable(GL_SCISSOR_TEST);
149-
m_fbo->release();
150161

151162
m_textureDirty = true;
152163
m_boundsDirty = true;
@@ -163,10 +174,7 @@ void scratchcpprender::PenLayer::drawLine(const PenAttributes &penAttributes, do
163174
if (!m_fbo || !m_painter || !m_engine)
164175
return;
165176

166-
// Begin painting
167-
m_fbo->bind();
168-
169-
m_painter->beginFrame(m_fbo->width(), m_fbo->height());
177+
Q_ASSERT(m_fbo->isBound());
170178

171179
// Apply scale (HQ pen)
172180
x0 *= m_scale;
@@ -205,10 +213,6 @@ void scratchcpprender::PenLayer::drawLine(const PenAttributes &penAttributes, do
205213
m_painter->stroke();
206214
}
207215

208-
// End painting
209-
m_painter->endFrame();
210-
m_fbo->release();
211-
212216
m_textureDirty = true;
213217
m_boundsDirty = true;
214218
update();
@@ -219,6 +223,8 @@ void PenLayer::stamp(IRenderedTarget *target)
219223
if (!target || !m_fbo || !m_texture.isValid() || m_vao == 0 || m_vbo == 0)
220224
return;
221225

226+
Q_ASSERT(m_fbo->isBound());
227+
222228
const float stageWidth = m_engine->stageWidth() * m_scale;
223229
const float stageHeight = m_engine->stageHeight() * m_scale;
224230

@@ -322,7 +328,9 @@ void PenLayer::stamp(IRenderedTarget *target)
322328
shaderProgram->release();
323329
m_glF->glBindVertexArray(0);
324330
m_glF->glBindBuffer(GL_ARRAY_BUFFER, 0);
325-
m_glF->glBindFramebuffer(GL_FRAMEBUFFER, 0);
331+
332+
// The FBO should remain bound until the frame ends
333+
// m_glF->glBindFramebuffer(GL_FRAMEBUFFER, 0);
326334

327335
m_glF->glEnable(GL_SCISSOR_TEST);
328336
m_glF->glEnable(GL_DEPTH_TEST);
@@ -395,9 +403,18 @@ QRgb PenLayer::colorAtScratchPoint(double x, double y) const
395403
if ((x < 0 || x >= width) || (y < 0 || y >= height))
396404
return qRgba(0, 0, 0, 0);
397405

406+
bool bound = m_fbo->isBound();
407+
408+
if (bound)
409+
const_cast<PenLayer *>(this)->endFrame();
410+
398411
GLubyte *data = m_textureManager.getTextureData(m_texture);
399412
const int index = (y * width + x) * 4; // RGBA channels
400413
Q_ASSERT(index >= 0 && index < width * height * 4);
414+
415+
if (bound)
416+
const_cast<PenLayer *>(this)->beginFrame();
417+
401418
return qRgba(data[index], data[index + 1], data[index + 2], data[index + 3]);
402419
}
403420

@@ -420,8 +437,17 @@ const libscratchcpp::Rect &PenLayer::getBounds() const
420437
const double width = m_texture.width();
421438
const double height = m_texture.height();
422439
std::vector<QPoint> points;
440+
441+
bool bound = m_fbo->isBound();
442+
443+
if (bound)
444+
const_cast<PenLayer *>(this)->endFrame();
445+
423446
m_textureManager.getTextureConvexHullPoints(m_texture, QSize(), ShaderManager::Effect::NoEffect, {}, points);
424447

448+
if (bound)
449+
const_cast<PenLayer *>(this)->beginFrame();
450+
425451
if (points.empty()) {
426452
m_bounds = libscratchcpp::Rect();
427453
return m_bounds;

‎src/penlayer.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ class PenLayer : public IPenLayer
3131
libscratchcpp::IEngine *engine() const override;
3232
void setEngine(libscratchcpp::IEngine *newEngine) override;
3333

34+
void beginFrame() override;
35+
void endFrame() override;
36+
3437
bool hqPen() const;
3538
void setHqPen(bool newHqPen);
3639

‎src/projectloader.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "valuemonitormodel.h"
1313
#include "listmonitormodel.h"
1414
#include "renderedtarget.h"
15+
#include "penlayer.h"
1516
#include "blocks/penblocks.h"
1617

1718
using namespace scratchcpprender;
@@ -231,7 +232,10 @@ void ProjectLoader::timerEvent(QTimerEvent *event)
231232

232233
m_unpositionedMonitors.clear();
233234

235+
IPenLayer *penLayer = PenLayer::getProjectPenLayer(m_engine);
236+
penLayer->beginFrame();
234237
m_engine->step();
238+
penLayer->endFrame();
235239

236240
if (m_running != m_engine->isRunning()) {
237241
m_running = !m_running;

‎test/mocks/penlayermock.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ class PenLayerMock : public IPenLayer
1818
MOCK_METHOD(libscratchcpp::IEngine *, engine, (), (const, override));
1919
MOCK_METHOD(void, setEngine, (libscratchcpp::IEngine *), (override));
2020

21+
MOCK_METHOD(void, beginFrame, (), (override));
22+
MOCK_METHOD(void, endFrame, (), (override));
23+
2124
MOCK_METHOD(void, clear, (), (override));
2225
MOCK_METHOD(void, drawPoint, (const PenAttributes &, double, double), (override));
2326
MOCK_METHOD(void, drawLine, (const PenAttributes &, double, double, double, double), (override));

‎test/penlayer/penlayer_test.cpp‎

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,10 @@ TEST_F(PenLayerTest, Clear)
231231
painter.endFrame();
232232
fbo->release();
233233

234+
penLayer.beginFrame();
234235
penLayer.clear();
236+
penLayer.endFrame();
237+
235238
QImage image2 = fbo->toImage();
236239

237240
// The image must contain only fully transparent pixels
@@ -275,7 +278,9 @@ TEST_F(PenLayerTest, DrawPoint)
275278
penLayer.drawPoint(attr, -54, 21);
276279
};
277280

281+
penLayer.beginFrame();
278282
draw();
283+
penLayer.endFrame();
279284

280285
{
281286
QOpenGLFramebufferObject *fbo = penLayer.framebufferObject();
@@ -302,8 +307,10 @@ TEST_F(PenLayerTest, DrawPoint)
302307
}
303308

304309
// Test HQ pen - draw
310+
penLayer.beginFrame();
305311
penLayer.clear();
306312
draw();
313+
penLayer.endFrame();
307314

308315
{
309316
QOpenGLFramebufferObject *fbo = penLayer.framebufferObject();
@@ -324,7 +331,8 @@ TEST_F(PenLayerTest, DrawLine)
324331
penLayer.setWidth(480);
325332
penLayer.setHeight(360);
326333
EngineMock engine;
327-
EXPECT_CALL(engine, stageWidth()).WillOnce(Return(480));
334+
EXPECT_CALL(engine, stageWidth()).WillRepeatedly(Return(480));
335+
EXPECT_CALL(engine, stageHeight()).WillRepeatedly(Return(360));
328336
penLayer.setEngine(&engine);
329337

330338
auto draw = [&penLayer]() {
@@ -350,7 +358,9 @@ TEST_F(PenLayerTest, DrawLine)
350358
penLayer.drawLine(attr, -54, 21, 88, -6);
351359
};
352360

361+
penLayer.beginFrame();
353362
draw();
363+
penLayer.endFrame();
354364

355365
{
356366
QOpenGLFramebufferObject *fbo = penLayer.framebufferObject();
@@ -364,12 +374,13 @@ TEST_F(PenLayerTest, DrawLine)
364374
}
365375

366376
// Test HQ pen
367-
penLayer.clear();
368-
EXPECT_CALL(engine, stageWidth()).Times(3).WillRepeatedly(Return(480));
369377
penLayer.setHqPen(true);
370378
penLayer.setWidth(720);
371379
penLayer.setHeight(540);
380+
penLayer.beginFrame();
381+
penLayer.clear();
372382
draw();
383+
penLayer.endFrame();
373384

374385
{
375386
QOpenGLFramebufferObject *fbo = penLayer.framebufferObject();
@@ -429,9 +440,13 @@ TEST_F(PenLayerTest, Stamp)
429440
i++;
430441
}
431442

443+
penLayer.beginFrame();
444+
432445
for (const auto &target : targets)
433446
penLayer.stamp(target.get());
434447

448+
penLayer.endFrame();
449+
435450
{
436451
QOpenGLFramebufferObject *fbo = penLayer.framebufferObject();
437452
QImage image = fbo->toImage().scaled(240, 180);
@@ -440,14 +455,17 @@ TEST_F(PenLayerTest, Stamp)
440455
}
441456

442457
// Test HQ pen
443-
penLayer.clear();
444458
penLayer.setHqPen(true);
445459
penLayer.setWidth(720);
446460
penLayer.setHeight(540);
461+
penLayer.beginFrame();
462+
penLayer.clear();
447463

448464
for (const auto &target : targets)
449465
penLayer.stamp(target.get());
450466

467+
penLayer.endFrame();
468+
451469
{
452470
QOpenGLFramebufferObject *fbo = penLayer.framebufferObject();
453471
QImage image = fbo->toImage();
@@ -464,8 +482,11 @@ TEST_F(PenLayerTest, TextureData)
464482
penLayer.setAntialiasingEnabled(false);
465483
EngineMock engine;
466484
EXPECT_CALL(engine, stageWidth()).WillRepeatedly(Return(6));
485+
EXPECT_CALL(engine, stageHeight()).WillRepeatedly(Return(4));
467486
penLayer.setEngine(&engine);
468487

488+
penLayer.beginFrame();
489+
469490
PenAttributes attr;
470491
attr.color = QNanoColor(255, 0, 0);
471492
attr.diameter = 1;
@@ -529,13 +550,18 @@ TEST_F(PenLayerTest, TextureData)
529550
ASSERT_EQ(bounds.right(), 2);
530551
ASSERT_EQ(bounds.bottom(), -2);
531552

553+
penLayer.endFrame();
554+
532555
// Test HQ pen
533-
penLayer.clear();
534-
EXPECT_CALL(engine, stageWidth()).Times(3).WillRepeatedly(Return(480));
535556
penLayer.setHqPen(true);
536557
penLayer.setWidth(720);
537558
penLayer.setHeight(540);
538559

560+
penLayer.beginFrame();
561+
penLayer.clear();
562+
EXPECT_CALL(engine, stageWidth()).WillRepeatedly(Return(480));
563+
EXPECT_CALL(engine, stageHeight()).WillRepeatedly(Return(360));
564+
539565
attr = PenAttributes();
540566
attr.color = QNanoColor(255, 0, 0);
541567
attr.diameter = 1;
@@ -545,10 +571,10 @@ TEST_F(PenLayerTest, TextureData)
545571
ASSERT_EQ(penLayer.colorAtScratchPoint(-1, 1), qRgb(255, 0, 0));
546572

547573
bounds = penLayer.getBounds();
548-
ASSERT_EQ(std::round(bounds.left() * 100) / 100, -3.33);
549-
ASSERT_EQ(bounds.top(), 2);
550-
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 3.67);
551-
ASSERT_EQ(bounds.bottom(), -3);
574+
ASSERT_EQ(std::round(bounds.left() * 100) / 100, -3);
575+
ASSERT_EQ(bounds.top(), 2.25);
576+
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 3.99);
577+
ASSERT_EQ(std::round(bounds.bottom() * 100) / 100, -3.24);
552578

553579
attr.color = QNanoColor(0, 128, 0, 128);
554580
attr.diameter = 2;
@@ -558,10 +584,10 @@ TEST_F(PenLayerTest, TextureData)
558584
ASSERT_EQ(penLayer.colorAtScratchPoint(-1, 1), qRgb(255, 0, 0));
559585

560586
bounds = penLayer.getBounds();
561-
ASSERT_EQ(std::round(bounds.left() * 100) / 100, -3.33);
562-
ASSERT_EQ(std::round(bounds.top() * 100) / 100, 2.67);
563-
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 4.33);
564-
ASSERT_EQ(std::round(bounds.bottom() * 100) / 100, -3.67);
587+
ASSERT_EQ(std::round(bounds.left() * 100) / 100, -3);
588+
ASSERT_EQ(std::round(bounds.top() * 100) / 100, 2.25);
589+
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 3.99);
590+
ASSERT_EQ(std::round(bounds.bottom() * 100) / 100, -3.24);
565591

566592
penLayer.clear();
567593
ASSERT_EQ(penLayer.colorAtScratchPoint(-3, 2), 0);
@@ -582,10 +608,10 @@ TEST_F(PenLayerTest, TextureData)
582608
ASSERT_EQ(penLayer.colorAtScratchPoint(-3, 1), qRgba(0, 0, 0, 0));
583609

584610
bounds = penLayer.getBounds();
585-
ASSERT_EQ(bounds.left(), 0);
586-
ASSERT_EQ(std::round(bounds.top() * 100) / 100, 1.33);
587-
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 1.67);
588-
ASSERT_EQ(std::round(bounds.bottom() * 100) / 100, -1.67);
611+
ASSERT_EQ(bounds.left(), -0.5);
612+
ASSERT_EQ(std::round(bounds.top() * 100) / 100, 1.5);
613+
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 2.49);
614+
ASSERT_EQ(std::round(bounds.bottom() * 100) / 100, -2.49);
589615

590616
attr.diameter = 2;
591617
penLayer.drawPoint(attr, -2, 0);
@@ -594,8 +620,10 @@ TEST_F(PenLayerTest, TextureData)
594620
ASSERT_EQ(penLayer.colorAtScratchPoint(-3, 1), 0);
595621

596622
bounds = penLayer.getBounds();
597-
ASSERT_EQ(std::round(bounds.left() * 100) / 100, -2.67);
598-
ASSERT_EQ(std::round(bounds.top() * 100) / 100, 1.33);
599-
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 1.67);
600-
ASSERT_EQ(std::round(bounds.bottom() * 100) / 100, -1.67);
623+
ASSERT_EQ(std::round(bounds.left() * 100) / 100, -3);
624+
ASSERT_EQ(std::round(bounds.top() * 100) / 100, 1.5);
625+
ASSERT_EQ(std::round(bounds.right() * 100) / 100, 2.49);
626+
ASSERT_EQ(std::round(bounds.bottom() * 100) / 100, -2.49);
627+
628+
penLayer.endFrame();
601629
}

0 commit comments

Comments
 (0)