[go: up one dir, main page]

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

add ShrinkTo to ppf and remove some CodecInOut in passes_test #3862

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mo271
Copy link
Member
@mo271 mo271 commented Sep 27, 2024

No description provided.

@mo271 mo271 added the CI:full Label to attach to a PR to run the full CI workflow and not just the regular PR workflows label Sep 27, 2024
@mo271 mo271 requested a review from szabadka September 27, 2024 12:59
@mo271
Copy link
Member Author
mo271 commented Sep 27, 2024

after this there are only 11 uses of CodecInOut::ShrinkTo left.


// Copy data row by row
for (size_t y = 0; y < new_ysize; ++y) {
memcpy(reinterpret_cast<uint8_t*>(new_pixels.get()) + y * new_stride,
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to reallocate + copy here? wouldn't leaving stride as is and just setting xsize/ysize to the new values work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess yes, but I thought after we reallocate and copy here, less memory is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the CodecIO/ImageF case we only set the new size, and don't reallocate. I think the idea was that this operation should be cheap.

Since this is only used in tests, one more reason to just set the new size is to find any cases where we incorrectly assume that stride = xsize * pixel_stride_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI:full Label to attach to a PR to run the full CI workflow and not just the regular PR workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants