Skip to content

CATROID-1646 Restore full-resolution session screenshots for visual placement#5194

Merged
urkat merged 8 commits into
Catrobat:developfrom
wslany:codex/visual-placement-screenshot-regression
Apr 13, 2026
Merged

CATROID-1646 Restore full-resolution session screenshots for visual placement#5194
urkat merged 8 commits into
Catrobat:developfrom
wslany:codex/visual-placement-screenshot-regression

Conversation

@wslany

@wslany wslany commented Apr 10, 2026

Copy link
Copy Markdown
Member

Visual placement uses the saved stage screenshot as its background image. Since #4852, that screenshot has been reduced to thumbnail resolution before being written, which makes thin pen/plot/cut/engrave lines disappear or look fuzzy in visual placement.

https://catrobat.atlassian.net/browse/CATROID-1646

This PR keeps the persisted screenshot thumbnail-sized so project storage and exported .catrobat files stay small, but also writes a full-resolution screenshot to app cache for the current session. Visual placement now prefers that session-local cache image.

It also adds a regression test that verifies the intended split:

  • persisted project/scene screenshots stay low-resolution
  • session-local screenshots keep the original framebuffer size for visual placement

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

  • Include the name of the Jira ticket in the PR’s title
  • Include a summary of the changes plus the relevant context
  • Choose the proper base branch (develop)
  • Confirm that the changes follow the project’s coding guidelines
  • Verify that the changes generate no compiler or linter warnings
  • Perform a self-review of the changes
  • Verify to commit no other files than the intentionally changed ones
  • Include reasonable and readable tests verifying the added or changed behavior
  • Confirm that new and existing unit tests pass locally
  • Check that the commits’ message style matches the project’s guideline
  • Stick to the project’s gitflow workflow
  • Verify that your changes do not have any conflicts with the base branch
  • After the PR, verify that all CI checks have passed
  • Post a message in the catroid-stage or catroid-ide Slack channel and ask for a code reviewer

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores full-resolution stage screenshots for visual placement by writing an additional session-local screenshot into the app cache while keeping persisted project/scene screenshots thumbnail-sized to avoid inflating project storage and exports.

Changes:

  • Added a session screenshot cache path and made visual placement background loading prefer session-local full-res screenshots.
  • Updated screenshot saving to write both persisted thumbnail screenshots and an extra full-resolution session screenshot to cache.
  • Added an Android regression test verifying persisted screenshots remain low-res while session-local cache screenshots keep framebuffer resolution.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
catroid/src/main/java/org/catrobat/catroid/utils/ProjectManagerExtensions.kt Adds session screenshot file resolution and preference logic in getProjectBitmap().
catroid/src/main/java/org/catrobat/catroid/stage/ScreenshotSaver.kt Writes an additional full-resolution screenshot to cache and refactors bitmap saving into a helper.
catroid/src/androidTest/java/org/catrobat/catroid/test/stage/ScreenshotSaverResolutionRegressionTest.kt Adds regression coverage for persisted-vs-session screenshot resolution behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread catroid/src/main/java/org/catrobat/catroid/utils/ProjectManagerExtensions.kt Outdated
Comment thread catroid/src/main/java/org/catrobat/catroid/stage/ScreenshotSaver.kt
@wslany

wslany commented Apr 10, 2026

Copy link
Copy Markdown
Member Author

Addressed the SonarCloud reliability finding from the latest Quality Gate failure in 20302ef17e8d6696a4353ddff08e22c3c67a16ea.

The issue was the ignored boolean return value of file.delete() in ScreenshotSaver.saveBitmap(). The code now checks the delete result explicitly and logs when cleanup of an incomplete screenshot file fails before throwing the IOException.

Local verification:

  • :catroid:compileCatroidDebugKotlin
  • :catroid:compileCatroidDebugJavaWithJavac
  • :catroid:compileCatroidDebugAndroidTestKotlin

@wslany wslany marked this pull request as ready for review April 10, 2026 12:35
@wslany wslany added the Active Member Tickets that are assigned to members that are still currently active label Apr 11, 2026
@harshsomankar123-tech

harshsomankar123-tech commented Apr 11, 2026

Copy link
Copy Markdown
Member

Thanks! @wslany for the changes .
I re-ran the new instrumentation test on the current PR head. It currently fails before the assertions run because StageActivity is auto-launched without a prepared current project/scene, so setUp() dereferences projectManager.currentlyPlayingScene while it is null. tearDown() then adds a secondary lateinit failure, and LibGDX teardown can also crash afterward.

'void com.badlogic.gdx.backends.android.keyboardheight.KeyboardHeightProvider.close()'
on a null object reference
at com.badlogic.gdx.backends.android.AndroidApplication.onDestroy()

It seems the activity can end up only partially initialized in the test environment, leading to a crash before the screenshot assertions complete. This doesn’t appear related to the screenshot logic itself, but it does mean the regression test is currently not passing as an instrumented test on my setup.
Please Take A Look and solve it.

Screenshot 2026-04-11 at 17 29 27

@wslany

wslany commented Apr 12, 2026

Copy link
Copy Markdown
Member Author

@harshsomankar123-tech Thanks again for catching the test-harness failure.

I addressed it in 8d6b148e50cc9b48d3f267202fd1acebc9840b85. The regression test no longer auto-launches StageActivity; it now prepares its own test project/current scene/current playing scene and uses the application context for DefaultAndroidFiles. tearDown() is also guarded so a partial setup cannot cause the secondary lateinit failure. This removes the partially initialized StageActivity/LibGDX teardown path from this regression test and keeps it focused on ScreenshotSaver plus getProjectBitmap() behavior.

Local emulator verification now passes on Medium_Phone_API_35 / emulator-5554:

adb shell am instrument -w -r -e class org.catrobat.catroid.test.stage.ScreenshotSaverResolutionRegressionTest org.catrobat.catroid.test/org.catrobat.catroid.runner.UiTestApplicationRunner

Time: 5.611
OK (3 tests)

I also rebuilt and installed the app/test APKs locally before running the instrumentation command:

./gradlew.bat :catroid:assembleCatroidDebug :catroid:assembleCatroidDebugAndroidTest --no-daemon --console=plain
BUILD SUCCESSFUL

@harshsomankar123-tech harshsomankar123-tech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wslany Thanks! LGTM

@sonarqubecloud

Copy link
Copy Markdown

@urkat urkat left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@urkat urkat merged commit bee5a23 into Catrobat:develop Apr 13, 2026
12 of 16 checks passed
@wslany wslany deleted the codex/visual-placement-screenshot-regression branch April 13, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Active Member Tickets that are assigned to members that are still currently active

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants