Fix Deep Scanline Input crash when using a framebuffer as parameter #2019
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Does this reveal a hole in our testing? Is there a test that can be added to validate the fix? Does the same bug exist for deep tiled input? |
Yes, there seems to be a hole in testing, since these overloaded functions are not being used in any of the existing tests. I will give it a go to add a case to one of the existing tests that uses these functions to extend our coverage. I think that DeepTiledInput is not affected by this issue, since it seems to be using a different code path, and does not seem to use the problematic function (readMemData). |
nikos-foundry
force-pushed
the
foundry-fix-deep-scanline-input-crash
branch
from
58e6484 to
9720aab
Compare
…ebuffer The Deep Scanline Input code has recently been rewritten to make use of the EXR core library. As a result a bug was introduced when using the overloads of readPixels() and readPixelSampleCounts() that use externally provided framebuffers as parameters, a hard crash was occuring. According to the comment on the header of these functions, when these overloads are used the InputFile's frameBuffer is not used. However both these functions call into readMemData(), which does use the InputFile's member framebuffer, which has not been allocated, resulting into a segmentation fault. This commit fixes the above issue, by using the parameter framebuffer that is passed in, which should be allocated by the caller application. Signed-off-by: Nikolaos Koutsikos <[email protected]>
After having found a bug with the readPixels() and readPixelSampleCounts() overloads that receive a framebuffer parameter, it was made apparent that there was a hole in our tests to cover these functions. This commits slightly refactors the DeepScanLineBasic tests and adds a third read path to add coverage for this case as well. Signed-off-by: Nikolaos Koutsikos <[email protected]>
nikos-foundry
force-pushed
the
foundry-fix-deep-scanline-input-crash
branch
from
9720aab to
7ca2ed0
Compare
peterhillman
approved these changes
May 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Deep Scanline Input code has recently been rewritten to make use of the EXR core library. As a result a bug was introduced when using the overloads of readPixels() and readPixelSampleCounts() that use externally provided framebuffers as parameters, a hard crash was occuring. According to the comment on the header of these functions, when these overloads are used the InputFile's frameBuffer is not used.
However both these functions call into readMemData(), which does use the InputFile's member framebuffer, which has not been allocated, resulting into a segmentation fault.
This commit fixes the above issue, by using the parameter framebuffer that is passed in, which should be allocated by the caller application.
Closes #2020