Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't interact with the transform layer if the image is a single layer #8149

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Districh-ru
Copy link
Collaborator

There is no need to set new data for the transform layer if the image is already in single layer mode - the transform layer will never be accessed.

@Districh-ru Districh-ru added the improvement New feature, request or improvement label Dec 17, 2023
@Districh-ru Districh-ru added this to the 1.0.11 milestone Dec 17, 2023
@Districh-ru Districh-ru self-assigned this Dec 17, 2023
@ihhub
Copy link
Owner

ihhub commented Dec 18, 2023

Hi @Districh-ru , let's target this pull request for the next release. I think we might need to add assertions to verify this changes.

@Districh-ru Districh-ru modified the milestones: 1.0.11, 1.1.0 Dec 18, 2023
@Districh-ru
Copy link
Collaborator Author

Hi @Districh-ru , let's target this pull request for the next release. I think we might need to add assertions to verify this changes.

Hi, @ihhub, sure, let's postpone it to v. 1.1.0.
These changes in example should speed-up the radar rendering because it uses fill() to render the single layer image.

@ihhub
Copy link
Owner

ihhub commented Dec 24, 2023

Hi @Districh-ru , I hit the assertion in Credits within Resize() function call.

@Districh-ru
Copy link
Collaborator Author

Hi @Districh-ru , I hit the assertion in Credits within Resize() function call.

Thanks, I'll check it. Previously I checked only the gameplay :)

@ihhub
Copy link
Owner

ihhub commented Dec 24, 2023

Hi @Districh-ru , I hit the assertion in Credits within Resize() function call.

Thanks, I'll check it. Previously I checked only the gameplay :)

Hi @Districh-ru , I believe we will need to rewrite Resize() function for this matter. I recommend not to spend too much time at the moment doing this unless a fix is very trivial.

@Districh-ru
Copy link
Collaborator Author

Hi @Districh-ru , I hit the assertion in Credits within Resize() function call.

Thanks, I'll check it. Previously I checked only the gameplay :)

Hi @Districh-ru , I believe we will need to rewrite Resize() function for this matter. I recommend not to spend too much time at the moment doing this unless a fix is very trivial.

Yes a fix is to make resizedOutput single-layer in game_credits.cpp before calling Resize().

But I plan to rewrite Resize today. :)

@ihhub
Copy link
Owner

ihhub commented Dec 24, 2023

@Districh-ru , then cool :)

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
src/engine/image.cpp Outdated Show resolved Hide resolved
@ihhub
Copy link
Owner

ihhub commented Dec 24, 2023

Hi @Districh-ru , it looks good but I will play around for a couple of days to make sure that no other places hit the assertion.

@Districh-ru
Copy link
Collaborator Author

Hi @Districh-ru , it looks good but I will play around for a couple of days to make sure that no other places hit the assertion.

Sure, I'll do the same.
I quickly checked image.cpp and all other functions there should work properly with single-layer images. Resize() is the only that needs a fix (and is fixed in this PR).

@ihhub ihhub modified the milestones: 1.0.12, 1.1.0 Feb 7, 2024
@ihhub ihhub modified the milestones: 1.1.0, 1.1.1 May 22, 2024
@ihhub ihhub modified the milestones: 1.1.1, 1.1.2 Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement New feature, request or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants