From e7813c2ff01e6a40f756d6a3d6ca94a8e856a6cd Mon Sep 17 00:00:00 2001 From: omigamedev Date: Sat, 13 Jun 2026 18:18:42 +0200 Subject: [PATCH] Extract stroke commit callback orchestration --- docs/modernization/debt.md | 5 + docs/modernization/tasks.md | 8 +- src/canvas.cpp | 103 +++++++++++---------- src/legacy_canvas_stroke_commit_services.h | 56 +++++++++++ tests/paint_renderer/compositor_tests.cpp | 85 +++++++++++++++++ 5 files changed, 206 insertions(+), 51 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 5c96296..d20b161 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,11 @@ agent or engineer to remove them without reconstructing context from chat. ## Recent Reductions +- 2026-06-13: DEBT-0036 was narrowed again. `Canvas::stroke_commit()` now + builds its retained callback table through + `make_legacy_canvas_stroke_commit_callbacks(...)`; the legacy executor still + owns ordering, history capture, and dilate sequencing, and the concrete + Canvas state wiring remains open in the legacy path. - 2026-06-13: DEBT-0036 was narrowed again. `Canvas::draw_merge()` now routes per-plane merge-target clear, blend-state gating, and optional checkerboard prepass through `execute_legacy_canvas_draw_merge_plane_setup(...)`; diff --git a/docs/modernization/tasks.md b/docs/modernization/tasks.md index 668871f..8ae4231 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -989,7 +989,7 @@ ctest --preset desktop-fast --build-config Debug -R "pp_paint_renderer_stroke_ex ### STR-009 - Extract Stroke Commit Callback Orchestration -Status: Ready +Status: Done Score: +2 renderer boundary and OpenGL parity Debt: `DEBT-0036` Scope: `src/canvas.cpp`, `src/legacy_canvas_stroke_execution_services.h`, @@ -1016,3 +1016,9 @@ Validation: ctest --preset desktop-fast --build-config Debug -R "pp_paint_renderer_stroke_execution" --output-on-failure & 'C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Current\Bin\MSBuild.exe' out\build\windows-msvc-default\tests\pp_paint_renderer_stroke_execution_tests.vcxproj /p:Configuration=Debug /p:Platform=x64 ``` + +Completed Task Log: + +| Date | Task | Score | Validation | Commit | +| --- | --- | ---: | --- | --- | +| 2026-06-13 | STR-009 | +2 renderer boundary and OpenGL parity | `ctest --preset desktop-fast --build-config Debug -R "retained_stroke_commit_callback_builder_preserves_order|retained_stroke_commit_runner_preserves_per_face_step_order" --output-on-failure`; `& 'C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Current\Bin\MSBuild.exe' out\build\windows-msvc-default\tests\pp_paint_renderer_compositor_tests.vcxproj /p:Configuration=Debug /p:Platform=x64` | `pending` | diff --git a/src/canvas.cpp b/src/canvas.cpp index 667d0e8..228fb64 100644 --- a/src/canvas.cpp +++ b/src/canvas.cpp @@ -392,11 +392,13 @@ void Canvas::stroke_draw_mix(const glm::vec2& bb_min, const glm::vec2& bb_sz) gl.save(); const auto layer_index = m_current_layer_idx; auto& current_layer = *m_layers[layer_index]; + std::array plane_transform {}; + std::copy(std::begin(m_plane_transform), std::end(m_plane_transform), plane_transform.begin()); const auto mix_planes = pp::panopainter::plan_legacy_canvas_stroke_mix_pass_planes( current_layer.m_visible, current_layer.m_opacity, glm::scale(glm::vec3(1, -1, 1)) * m_proj * m_mv, - m_plane_transform, + plane_transform, [&](int plane_index) { return current_layer.face(plane_index); }); @@ -757,8 +759,7 @@ void Canvas::stroke_draw() [&] { m_sampler.unbind(); }); - const pp::panopainter::LegacyCanvasStrokeTextureInputDispatch main_pass_texture_dispatch { - pp::panopainter::make_legacy_canvas_stroke_main_pass_texture_dispatch( + const auto main_pass_texture_dispatch = pp::panopainter::make_legacy_canvas_stroke_main_pass_texture_dispatch( [&](int texture_slot) { set_active_texture_unit(texture_slot); }, @@ -845,7 +846,7 @@ void Canvas::stroke_draw() .input = pp::panopainter::LegacyCanvasStrokeTextureInput::stroke_destination, .slot = 1, }, - }; + }; const auto make_pad_destination_texture_dispatch = [&](int face_index) { return pp::panopainter::make_legacy_canvas_stroke_pad_destination_texture_dispatch( [&](int texture_slot) { @@ -914,10 +915,6 @@ void Canvas::stroke_draw() pp::panopainter::execute_legacy_canvas_stroke_dual_pass( pp::panopainter::LegacyCanvasStrokeDualPassRequest { .context = "Canvas::stroke_draw", - .setup_dual_shader = [&] { - pp::panopainter::setup_legacy_stroke_dual_shader( - stroke_material.dual_pass.uses_pattern); - }, .bind_brush_tip = [&] { pp::panopainter::bind_legacy_canvas_stroke_texture_inputs( dual_pass_texture_bindings, @@ -936,6 +933,28 @@ void Canvas::stroke_draw() unbind_texture_2d(); })); }, + .unbind_brush_tip = [&] { + pp::panopainter::unbind_legacy_canvas_stroke_texture_inputs( + dual_pass_texture_bindings, + pp::panopainter::make_legacy_canvas_stroke_brush_tip_texture_dispatch( + [&](int texture_slot) { + set_active_texture_unit(texture_slot); + }, + [&] { + dual_brush->m_tip_texture ? + dual_brush->m_tip_texture->bind() : + unbind_texture_2d(); + }, + [&] { + dual_brush->m_tip_texture ? + dual_brush->m_tip_texture->unbind() : + unbind_texture_2d(); + })); + }, + .setup_dual_shader = [&] { + pp::panopainter::setup_legacy_stroke_dual_shader( + stroke_material.dual_pass.uses_pattern); + }, .execute_frame_pass = [&] { pp::panopainter::execute_legacy_canvas_stroke_dual_pass_frame_callbacks( frames_dual, @@ -958,24 +977,6 @@ void Canvas::stroke_draw() m_tmp_dual, true); }, - .unbind_brush_tip = [&] { - pp::panopainter::unbind_legacy_canvas_stroke_texture_inputs( - dual_pass_texture_bindings, - pp::panopainter::make_legacy_canvas_stroke_brush_tip_texture_dispatch( - [&](int texture_slot) { - set_active_texture_unit(texture_slot); - }, - [&] { - dual_brush->m_tip_texture ? - dual_brush->m_tip_texture->bind() : - unbind_texture_2d(); - }, - [&] { - dual_brush->m_tip_texture ? - dual_brush->m_tip_texture->unbind() : - unbind_texture_2d(); - })); - }, }); } @@ -1117,44 +1118,39 @@ void Canvas::stroke_commit() }; } - [[maybe_unused]] const auto commit_result = pp::panopainter::execute_legacy_canvas_stroke_commit_sequence( - pp::panopainter::LegacyCanvasStrokeCommitRequest { - .context = "Canvas::stroke_commit", - .faces = faces, - .sequence = sequence, - .callbacks = { - .mark_commit_started = [&]() { + const auto commit_callbacks = pp::panopainter::make_legacy_canvas_stroke_commit_callbacks( + [&]() { m_dirty = false; m_dirty_stroke = true; // new stroke ready for timelapse capture App::I->redraw = true; m_unsaved = true; App::I->title_update(); }, - .capture_render_state = []() {}, - .prepare_render_state = [&]() { + []() {}, + [&]() { apply_canvas_viewport(0, 0, m_width, m_height); apply_canvas_capability(blend_state(), false); }, - .restore_render_state = [&]() { + [&]() { blend ? apply_canvas_capability(blend_state(), true) : apply_canvas_capability(blend_state(), false); apply_canvas_viewport(vp.x, vp.y, vp.width, vp.height); apply_canvas_clear_color(cc); set_active_texture_unit(0); }, - .publish_history = [&]() { + [&]() { action->m_layer_idx = m_current_layer_idx; action->m_frame_idx = layer().m_frame_index; action->m_canvas = this; //action->m_stroke = std::move(m_current_stroke); ActionManager::add(action); }, - .capture_timelapse_frame = [&]() { + [&]() { stroke_commit_timelapse(); }, - .bind_layer_framebuffer = [&](int i) { + [&](int i) { m_layers[m_current_layer_idx]->rtt(i).bindFramebuffer(); }, - .capture_history_region = [&](int i) { + [&](int i) { // save image before commit glm::vec2 box_sz = zw(m_dirty_box[i]) - xy(m_dirty_box[i]); action->m_image[i] = std::make_unique( @@ -1170,7 +1166,7 @@ void Canvas::stroke_commit() action->m_old_box[i] = m_layers[m_current_layer_idx]->box(i); action->m_old_dirty[i] = m_layers[m_current_layer_idx]->face(i); }, - .apply_layer_dirty_region = [&](int i) { + [&](int i) { if (!m_layers[m_current_layer_idx]->m_alpha_locked) { auto& lbox = m_layers[m_current_layer_idx]->box(i); @@ -1181,14 +1177,14 @@ void Canvas::stroke_commit() } m_layers[m_current_layer_idx]->face(i) = true; }, - .copy_layer_to_commit_destination = [&](int i) { + [&](int i) { // copy to tmp2 for layer blending set_active_texture_unit(0); m_tex2[i].bind(); copy_framebuffer_to_texture_2d(0, 0, 0, 0, m_width, m_height); m_tex2[i].unbind(); }, - .bind_commit_inputs = [&](int i) { + [&](int i) { pp::panopainter::bind_legacy_canvas_stroke_commit_inputs( sequence, [&](int texture_slot) { @@ -1231,7 +1227,7 @@ void Canvas::stroke_commit() } }); }, - .execute_erase_composite = [&](int) { + [&](int) { pp::panopainter::execute_legacy_canvas_stroke_commit_erase( [&]() { pp::panopainter::setup_legacy_stroke_erase_shader( @@ -1248,7 +1244,7 @@ void Canvas::stroke_commit() m_plane.draw_fill(); }); }, - .execute_paint_composite = [&](int) { + [&](int) { glm::vec2 patt_scale = glm::vec2(b->m_pattern_scale); if (b->m_pattern_flipx) patt_scale.x *= -1.f; if (b->m_pattern_flipy) patt_scale.y *= -1.f; @@ -1283,7 +1279,7 @@ void Canvas::stroke_commit() m_plane.draw_fill(); }); }, - .copy_committed_to_dilate_source = [&](int i) { + [&](int i) { pp::panopainter::copy_legacy_canvas_stroke_commit_to_dilate_source( sequence, [&]() { @@ -1307,15 +1303,22 @@ void Canvas::stroke_commit() .height = m_height, }); }, - .execute_commit_dilate = [&](int) { + [&](int) { pp::panopainter::execute_legacy_canvas_stroke_commit_dilate([&]() { m_plane.draw_fill(); }); }, - .unbind_layer_framebuffer = [&](int i) { + [&](int i) { m_layers[m_current_layer_idx]->rtt(i).unbindFramebuffer(); - }, - }, + } + ); + + [[maybe_unused]] const auto commit_result = pp::panopainter::execute_legacy_canvas_stroke_commit_sequence( + pp::panopainter::LegacyCanvasStrokeCommitRequest { + .context = "Canvas::stroke_commit", + .faces = faces, + .sequence = sequence, + .callbacks = commit_callbacks, }); } diff --git a/src/legacy_canvas_stroke_commit_services.h b/src/legacy_canvas_stroke_commit_services.h index dad356a..3defc34 100644 --- a/src/legacy_canvas_stroke_commit_services.h +++ b/src/legacy_canvas_stroke_commit_services.h @@ -7,6 +7,7 @@ #include #include #include +#include namespace pp::panopainter { @@ -52,6 +53,61 @@ struct LegacyCanvasStrokeCommitCopyExtent { int height = 0; }; +template < + typename MarkCommitStarted, + typename CaptureRenderState, + typename PrepareRenderState, + typename RestoreRenderState, + typename PublishHistory, + typename CaptureTimelapseFrame, + typename BindLayerFramebuffer, + typename CaptureHistoryRegion, + typename ApplyLayerDirtyRegion, + typename CopyLayerToCommitDestination, + typename BindCommitInputs, + typename ExecuteEraseComposite, + typename ExecutePaintComposite, + typename CopyCommittedToDilateSource, + typename ExecuteCommitDilate, + typename UnbindLayerFramebuffer> +[[nodiscard]] inline LegacyCanvasStrokeCommitCallbacks make_legacy_canvas_stroke_commit_callbacks( + MarkCommitStarted&& mark_commit_started, + CaptureRenderState&& capture_render_state, + PrepareRenderState&& prepare_render_state, + RestoreRenderState&& restore_render_state, + PublishHistory&& publish_history, + CaptureTimelapseFrame&& capture_timelapse_frame, + BindLayerFramebuffer&& bind_layer_framebuffer, + CaptureHistoryRegion&& capture_history_region, + ApplyLayerDirtyRegion&& apply_layer_dirty_region, + CopyLayerToCommitDestination&& copy_layer_to_commit_destination, + BindCommitInputs&& bind_commit_inputs, + ExecuteEraseComposite&& execute_erase_composite, + ExecutePaintComposite&& execute_paint_composite, + CopyCommittedToDilateSource&& copy_committed_to_dilate_source, + ExecuteCommitDilate&& execute_commit_dilate, + UnbindLayerFramebuffer&& unbind_layer_framebuffer) +{ + return LegacyCanvasStrokeCommitCallbacks { + .mark_commit_started = std::forward(mark_commit_started), + .capture_render_state = std::forward(capture_render_state), + .prepare_render_state = std::forward(prepare_render_state), + .restore_render_state = std::forward(restore_render_state), + .publish_history = std::forward(publish_history), + .capture_timelapse_frame = std::forward(capture_timelapse_frame), + .bind_layer_framebuffer = std::forward(bind_layer_framebuffer), + .capture_history_region = std::forward(capture_history_region), + .apply_layer_dirty_region = std::forward(apply_layer_dirty_region), + .copy_layer_to_commit_destination = std::forward(copy_layer_to_commit_destination), + .bind_commit_inputs = std::forward(bind_commit_inputs), + .execute_erase_composite = std::forward(execute_erase_composite), + .execute_paint_composite = std::forward(execute_paint_composite), + .copy_committed_to_dilate_source = std::forward(copy_committed_to_dilate_source), + .execute_commit_dilate = std::forward(execute_commit_dilate), + .unbind_layer_framebuffer = std::forward(unbind_layer_framebuffer), + }; +} + [[nodiscard]] inline std::size_t legacy_canvas_stroke_commit_step_count( const pp::paint_renderer::CanvasStrokeCommitSequencePlan& sequence) noexcept { diff --git a/tests/paint_renderer/compositor_tests.cpp b/tests/paint_renderer/compositor_tests.cpp index 1134080..49c56e5 100644 --- a/tests/paint_renderer/compositor_tests.cpp +++ b/tests/paint_renderer/compositor_tests.cpp @@ -2013,6 +2013,88 @@ void retained_stroke_commit_runner_preserves_per_face_step_order(pp::tests::Harn PP_EXPECT(h, events == expected); } +void retained_stroke_commit_callback_builder_preserves_order(pp::tests::Harness& h) +{ + const auto sequence = plan_canvas_stroke_commit_sequence( + CanvasStrokeCommitRequest { + .erase_mode = false, + .alpha_locked = false, + .selection_mask_active = true, + .dual_stroke_enabled = true, + .pattern_enabled = true, + }); + + std::vector events; + const auto record = [&](std::string_view event, int face_index = -1) { + if (face_index >= 0) { + events.push_back(std::string(event) + ":" + std::to_string(face_index)); + } else { + events.push_back(std::string(event)); + } + }; + + const auto callbacks = pp::panopainter::make_legacy_canvas_stroke_commit_callbacks( + [&]() { record("start"); }, + [&]() { record("capture"); }, + [&]() { record("prepare"); }, + [&]() { record("restore"); }, + [&]() { record("publish"); }, + [&]() { record("timelapse"); }, + [&](int face_index) { record("bind-fbo", face_index); }, + [&](int face_index) { record("history", face_index); }, + [&](int face_index) { record("dirty", face_index); }, + [&](int face_index) { record("copy-layer", face_index); }, + [&](int face_index) { record("bind-inputs", face_index); }, + [&](int face_index) { record("erase", face_index); }, + [&](int face_index) { record("paint", face_index); }, + [&](int face_index) { record("copy-committed", face_index); }, + [&](int face_index) { record("dilate", face_index); }, + [&](int face_index) { record("unbind-fbo", face_index); }); + + const auto result = pp::panopainter::execute_legacy_canvas_stroke_commit_sequence( + pp::panopainter::LegacyCanvasStrokeCommitRequest { + .context = "test", + .faces = { + pp::panopainter::LegacyCanvasStrokeCommitFace { .index = 0, .dirty = true }, + pp::panopainter::LegacyCanvasStrokeCommitFace { .index = 1, .dirty = false }, + pp::panopainter::LegacyCanvasStrokeCommitFace { .index = 2, .dirty = true }, + }, + .sequence = sequence, + .callbacks = callbacks, + }); + + const std::vector expected { + "start", + "capture", + "prepare", + "bind-fbo:0", + "history:0", + "dirty:0", + "copy-layer:0", + "bind-inputs:0", + "paint:0", + "copy-committed:0", + "dilate:0", + "unbind-fbo:0", + "bind-fbo:2", + "history:2", + "dirty:2", + "copy-layer:2", + "bind-inputs:2", + "paint:2", + "copy-committed:2", + "dilate:2", + "unbind-fbo:2", + "restore", + "publish", + "timelapse", + }; + + PP_EXPECT(h, result.ok); + PP_EXPECT(h, result.committed_faces == 2); + PP_EXPECT(h, events == expected); +} + void retained_stroke_commit_copy_skips_missing_layer_scratch_or_invalid_extent(pp::tests::Harness& h) { pp::paint_renderer::CanvasStrokeCommitSequencePlan missing_scratch; @@ -3020,6 +3102,9 @@ int main() harness.run( "retained_stroke_commit_runner_preserves_per_face_step_order", retained_stroke_commit_runner_preserves_per_face_step_order); + harness.run( + "retained_stroke_commit_callback_builder_preserves_order", + retained_stroke_commit_callback_builder_preserves_order); harness.run( "retained_stroke_commit_copy_skips_missing_layer_scratch_or_invalid_extent", retained_stroke_commit_copy_skips_missing_layer_scratch_or_invalid_extent);