Skip to content

Change offset to pixels#10

Closed
jonasteuwen wants to merge 2 commits intoamspath:mainfrom
NKI-AI:fix-offset
Closed

Change offset to pixels#10
jonasteuwen wants to merge 2 commits intoamspath:mainfrom
NKI-AI:fix-offset

Conversation

@jonasteuwen
Copy link
Copy Markdown
Collaborator

@jonasteuwen jonasteuwen commented Apr 20, 2023

This commit:

  • Changes offset_in_pixels to u64. In my tests this also corresponds to the actual size of the padding
  • Add offset also for level = 0
  • Add width and height properties to the level
  • Expose the height, width, mpp_x and mpp_y of each level in the public API.

It is forked from the changes I made to make a read_region function work.

Comment thread src/isyntax/isyntax.c
// (this is also reflected in the x and y coordinates of the codeblocks in the iSyntax header).
for (i32 scale = 0; scale < wsi_image->level_count; ++scale) {
isyntax_level_t* level = wsi_image->levels + scale;
level->origin_offset_in_pixels = ((PER_LEVEL_PADDING << wsi_image->level_count) - PER_LEVEL_PADDING) >> scale;
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.

This doesn't look right (see my other comment elsewhere).
To keep the git history clean I'll fixup and reproduce the changes from this commit in my own commit.

Comment thread src/isyntax/isyntax.h
i32 width_in_tiles;
i32 height_in_tiles;
u64 width;
u64 height;
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.

u64 doesn't match int32_t in the getters in libisyntax.c/h
I'll go with i32 here.

@Falcury
Copy link
Copy Markdown
Member

Falcury commented Apr 20, 2023

The git commit history became a bit messy and I found some errors that needed fixing, so I decided to make a new commit 82d04a6

@Falcury Falcury closed this Apr 20, 2023
@Falcury
Copy link
Copy Markdown
Member

Falcury commented Apr 20, 2023

Note that iterating from scale 1 is correct. The values should all be 0 for level 0, and will be incorrect if included in the for loop.
Should be fixed in 68400c3.

@jonasteuwen
Copy link
Copy Markdown
Collaborator Author

@Falcury Thanks for the "merge". Are you sure the offset isn't there in level 0? I checked with the isyntax_example.

@jonasteuwen jonasteuwen deleted the fix-offset branch April 20, 2023 20:33
@Falcury
Copy link
Copy Markdown
Member

Falcury commented Apr 20, 2023

The way I have it in my head is: level 0 has offset 0 (i.e. needs no initialization because already zero-initialized). Then each successive level has a higher offset (as calculated in terms of level 0 pixels) that follows the formula.

But, I also know I’ve gotten this stuff wrong in the past (multiple times)... I’ll run some tests tomorrow, including using your TIFF converter, and try to fix it if there are still errors!

@jonasteuwen
Copy link
Copy Markdown
Collaborator Author

jonasteuwen commented Apr 20, 2023

Great, thanks! We use SlideScore, which uses the Philips SDK, as a sanity check I will look up some slides we annotated there and compare with isyntax -> tiff in the different levels to see if they align in the same way.

I think in the dwt in level 0, what they do is have offset = 3*(2^n - 1), which drops by a factor of 2^scale each step so that at the end (lowest level) you end up at 3 * (2^n - 1)/2^(n-1) ~5, which is around the minimal padding required to be able to compute their dwt.

@Falcury
Copy link
Copy Markdown
Member

Falcury commented Apr 21, 2023

I think in the dwt in level 0, what they do is have offset = 3*(2^n - 1), which drops by a factor of 2^scale each step so that at the end (lowest level) you end up at 3 * (2^n - 1)/2^(n-1) ~5, which is around the minimal padding required to be able to compute their dwt.

Probably this gives 'approximately' correct results, but I'm almost certain that this is not fully correct. Philips' documentation on the file format also specifies the per level padding to be ((3 << level) - 2). I believe this refers to the encoding process, where level 0 is the base image and for each successive encoding step, padding is added to enable the wavelet transform to proceed correctly. For decoding, you have to 'think in reverse' as to what is going on and the actual offset for the decoded image ends up being ((3 << (level-1)) - 2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants