DecodePixelData incorrectly y flips the image when loading "Decreasing Y" files

[MarkCallow]

Describe the issue
I think the title is a clear and concise description.

To Reproduce
Steps to reproduce the behavior:

1. Use the simple test file, decreasing.exr, in testexrorder.zip attached below.
2. Call LoadEXRImageFromMemory to load the file.
3. Observe that the pixel on the first line is 1 (white/red). It should be black.

testexrorder.zip contains increasing and decreasing versions of the same simple 3 line image. See AcademySoftwareFoundation/openexr#1946 (comment) for a detailed description of the test case.

The same issue arises using the only decreasing Y file I found in the Academy's EXR test files: https://raw.githubusercontent.com/AcademySoftwareFoundation/openexr-images/main/ScanLines/Blobbies.jpg.

Expected behavior
I got this result:

I expected this

Ignore that it is red (the sole channel in the file is labelled "R" which my software follows) and the lighter area (this is from a 3D scene with illumination).

The images in "decreasing.exr" and "increasing.exr" are identical so the result should be identical.

** My Analysis **

The source data_ptr passed to DecodePixelData is acquired from offsets[y_idx] where y_idx essentially becomes the line_no parameter also passed to DecodePixelData. So data_ptr is pointing at the correct source for, e.g. line 0 so it just needs to be copied to the first scanline in the output image.

But in DecodePixelData we see lines like

tinyexr/tinyexr.h

Lines 4100 to 4105 in 5fcb4dc

	if (line_order == 0) {
	outLine += (size_t(y) + v) * size_t(x_stride);
	} else {
	outLine +=
	(size_t(height) - 1 - (size_t(y) + v)) * size_t(x_stride);
	}

The above is taken from the uncompressed case as being the simplest example.

We can see that for decreasing line order it sets the output pointer to the last scanline of the output and proceeds to write scanline 0 there.

I can workaround this by setting line_order = 0 in the header I pass to LoadEXRImageFromMemory. But I am worried this might not work for, lets say, more complex cases. In both the examples I've referenced DecodePixelData is called once for each scanline with num_lines == 1. I do not know enough about EXR or TinyEXR to judge what cases, if any, would be broken by simply removing the if (line_order == 0) clauses and always performing the line_order == 0 case. num_lines > 1 looks iffy to me.

Environment

• OS: macOS Sequoia
• Compiler Apple clang version 16.0.0 (clang-1600.0.26.6)

[syoyo]

Thank you for the detailed report.

Let me give some time to investigate the issue(and the spec of EXR lineOrder)

[MarkCallow]

I think that outline should always be set with

outLine += (size_t(y) + v) * size_t(x_stride);

because regardless of the order of the scanlines in the file, the scanline for y is the scanline for y. That is, the scanline for coord y is found in the file from entry y in the scanline offsets table. And the scanline for y will always be labelled as such.

For the case where num_lines > 1 I think the source pointer, data_ptr needs to be decremented for files with Decreasing Y scanline order (line_order == 1) and incremented for Increasing (line_order == 0).

If DecodePixelData is ever called with line_order == 2 (random) and num_lines > 1 the function will not work. Whether such a case is possible, I have no idea.

[syoyo]

Firstly, lineOrder=RANDOM_Y is only applicable to Tiled image, and TinyEXR does not support RANDOM_Y yet, so we'll discuss the issue for a scanline image with increasing_y and decreasing_y.

Dumping line_no info at here:

tinyexr/tinyexr.h

Line 5318 in 756f7d3

for Blobbies.exr gives the following result.

y 0, offsets[y] 6091998, line_no -20
y 1, offsets[y] 6066206, line_no -4
y 2, offsets[y] 6024978, line_no 12
y 3, offsets[y] 5969465, line_no 28
y 4, offsets[y] 5915415, line_no 44
y 5, offsets[y] 5855750, line_no 60
y 6, offsets[y] 5789599, line_no 76
y 7, offsets[y] 5716930, line_no 92
y 8, offsets[y] 5640285, line_no 108
y 9, offsets[y] 5554768, line_no 124
y 10, offsets[y] 5463955, line_no 140
y 11, offsets[y] 5369044, line_no 156
y 12, offsets[y] 5268097, line_no 172
y 13, offsets[y] 5162941, line_no 188
y 14, offsets[y] 5053789, line_no 204
y 15, offsets[y] 4940378, line_no 220
y 16, offsets[y] 4820515, line_no 236
y 17, offsets[y] 4700124, line_no 252
y 18, offsets[y] 4580597, line_no 268
y 19, offsets[y] 4465203, line_no 284
y 20, offsets[y] 4354738, line_no 300
y 21, offsets[y] 4242952, line_no 316
y 22, offsets[y] 4131968, line_no 332
y 23, offsets[y] 4027004, line_no 348
y 24, offsets[y] 3927168, line_no 364
y 25, offsets[y] 3828424, line_no 380
y 26, offsets[y] 3726571, line_no 396
y 27, offsets[y] 3620766, line_no 412
y 28, offsets[y] 3517113, line_no 428
y 29, offsets[y] 3413746, line_no 444
y 30, offsets[y] 3309347, line_no 460
y 31, offsets[y] 3201101, line_no 476
y 32, offsets[y] 3089596, line_no 492
y 33, offsets[y] 2982063, line_no 508
y 34, offsets[y] 2871086, line_no 524
y 35, offsets[y] 2750491, line_no 540
y 36, offsets[y] 2622890, line_no 556
y 37, offsets[y] 2494044, line_no 572
y 38, offsets[y] 2360190, line_no 588
y 39, offsets[y] 2226897, line_no 604
y 40, offsets[y] 2095490, line_no 620
y 41, offsets[y] 1972979, line_no 636
y 42, offsets[y] 1856220, line_no 652
y 43, offsets[y] 1745940, line_no 668
y 44, offsets[y] 1643646, line_no 684
y 45, offsets[y] 1543684, line_no 700
y 46, offsets[y] 1449368, line_no 716
y 47, offsets[y] 1358068, line_no 732
y 48, offsets[y] 1273503, line_no 748
y 49, offsets[y] 1199181, line_no 764
y 50, offsets[y] 1129002, line_no 780
y 51, offsets[y] 1062595, line_no 796
y 52, offsets[y] 994564, line_no 812
y 53, offsets[y] 925704, line_no 828
y 54, offsets[y] 850506, line_no 844
y 55, offsets[y] 768778, line_no 860
y 56, offsets[y] 682739, line_no 876
y 57, offsets[y] 593117, line_no 892
y 58, offsets[y] 503680, line_no 908
y 59, offsets[y] 415161, line_no 924
y 60, offsets[y] 332265, line_no 940
y 61, offsets[y] 250517, line_no 956
y 62, offsets[y] 172401, line_no 972
y 63, offsets[y] 103039, line_no 988
y 64, offsets[y] 41110, line_no 1004

NOTE: ZIP compression(used on Blobbies.exr) uses 16 scanlines(num_lines) per block.

So, i'th block(scanline) corresponds to i'th block(scanline) position(no change to the ordering of scanline) but offset(memory address) is descending order for decreasing Y.

Also, scanline data itself in the memory seems not flipped when num_lines > 1, so we probably don't need to consider line_order in DecodePixels as you pointed out!

[syoyo]

@MarkCallow Could you please prepare synthetic EXR image with decreasing Y, with compression none(num_lines = 1) and ZIP(num_lines=16) using OpenEXR lib?

e.g. height 256 image, whose y'th scanline value has 'y'.

[MarkCallow]

What is line_no in the table you list @syoyo? A negative line_nois making my head spin.

testexrorder.zip, which I uploaded in the initial description, has a file with compression none, exactly as you request except it has only 3 lines. It also includes the program used to create the file. It should be a simple matter to modify the program to write 256 lines and probably not much more difficult to make it zip compressed. I have never used the OpenEXR lib so I would have a severe learning curve. Please make the files yourself using the content of testexrorder.zip.

[syoyo]

You should read the comment here:

tinyexr/tinyexr.h

Line 5338 in 756f7d3

// line_no may be negative.

Blobbies.exr has negative value in dataWindow

dataWindow (type box2i): (-20 -20) - (1019 1019)

I have never used the OpenEXR lib

? testexrorder.zip has C++ code with OpenEXR lib.

[MarkCallow]

Blobbies.exr has negative value in dataWindow

Thanks. I'll have to study the data window. I'm new to EXR.

I have never used the OpenEXR lib

? testexrorder.zip has C++ code with OpenEXR lib.

I did not write that. It was written by @peterhillman and provided by him in AcademySoftwareFoundation/openexr#1946 (comment) to help explain decreasing scanline order to me.

[syoyo]

You should not post someone's code.

There is no meaningful info about lineOrder attribute in the OpenEXR spec, so you need to check the source code of OpenEXR.

After some code investigation, lineOrder handling when reading the image changed from openexr 2.0(TinyEXR is based on) and openexr 3.0.

For example, there is no lineOrder consideration anymore in recent ImfScanLineInputFile.cpp

https://github.com/AcademySoftwareFoundation/openexr/blob/main/src/lib/OpenEXR/ImfScanLineInputFile.cpp

Whereas lineOrder is considered in openexr 2.0

https://github.com/mitsuba-renderer/openexr/blob/dbabb6f9500ee628c1faba21bb8add2649cc32a6/OpenEXR/IlmImf/ImfScanLineInputFile.cpp#L571

So flipping may be an expected behavior in openexr 2.0.

You need to prepare reproducible code with openexr 2.0 and openexr 3.0, and check if how it behaves.