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

Incorrect JPG, PNG tile drawing (precise algorithm) #98

Open
adl1995 opened this issue Jul 31, 2017 · 8 comments
Open

Incorrect JPG, PNG tile drawing (precise algorithm) #98

adl1995 opened this issue Jul 31, 2017 · 8 comments
Assignees
Labels
Milestone

Comments

@adl1995
Copy link
Member

adl1995 commented Jul 31, 2017

I'm currently working on introducing precise drawing, the tile splitting works correctly, but, when I try and draw the all sky image, I end up with something like this:

precise-drawing

But, it works fine using the simple algorithm:

simple-drawing

Note: this issue is only for JPG and PNG tiles, the FITS tiles are being drawn correctly using the precise algorithm.

I tried displaying the JPG tile children individually, and I get this result:

four-child-tiles

The original tile is this:

single-tile

So, there is no issue with the children property as far as I can tell, my guess is that this has something to do with how the corners are being computed. The Jupyter notebook containing this code can be found here: https://github.com/adl1995/HIPS-to-Py/blob/master/examples/Precise%20drawing.ipynb

Since @cdeil is on vacation, @tboch can you please take a look at this?

@adl1995 adl1995 changed the title Incorrect JPG, PNG precise tile drawing Incorrect JPG, PNG tile drawing (precise algorithm) Jul 31, 2017
@tboch
Copy link
Member

tboch commented Jul 31, 2017

yes, I'll have a look at this ASAP.

@tboch
Copy link
Member

tboch commented Aug 1, 2017

@adl1995 : as JPEG/PNG tiles are vertically flipped with respect to FITS tiles, I think that the order of items in the children property has to be changed: for JPEG/PNG tiles, items at indices 0 and 1 must be swapped, same thing for items at indices 2 and 3.
Can you try this and report if it works fine?

@adl1995
Copy link
Member Author

adl1995 commented Aug 1, 2017

@tboch Did you mean changing the order of children tiles? You can see the updated code here: https://github.com/adl1995/hips/blob/precise/hips/tiles/tile.py#L252

But, this did not have any effect. Here is the updated notebook: https://github.com/adl1995/HIPS-to-Py/blob/master/examples/Precise%20drawing.ipynb

Also, the order of children tiles is now incorrect, as it can be seen in the last cell of the notebook.

@tboch
Copy link
Member

tboch commented Aug 1, 2017

yes, that's what I meant.
Let me have a closer look at this.

@tboch
Copy link
Member

tboch commented Aug 1, 2017

@adl1995 OK, I think I understand better now what happens now. Actually, when children tiles are created, the order of items in the data array must be changed as follow:

        data = [
            self.data[w: w * 2, 0: w],
            self.data[0: w, 0: w],
            self.data[w: w * 2, w: w * 2],
            self.data[0: w, w: w * 2]
        ]

This seems to be independent from the tile format

@adl1995
Copy link
Member Author

adl1995 commented Aug 1, 2017

@tboch Thanks! The drawing works now. I will open up a PR shortly.

@cdeil
Copy link
Contributor

cdeil commented Aug 3, 2017

as JPEG/PNG tiles are vertically flipped with respect to FITS tiles

@tboch - You've probably seen this by now, but just in case: in https://github.com/hipspy/hips/pull/90/files#diff-a61b34ea30f91431cdbfd58e3c54dd89R142 I changed tile I/O to always flip to the FITS orientation on I/O, i.e. within the HiPS package we have to do less work and not consider two cases or orientation any more. Do you think that approach is OK or is it bad for some reason? (clearly it's not optimal performance, but I think the performance overhead on memory / CPU should be very small compared to other things we do with tiles?)

@tboch
Copy link
Member

tboch commented Aug 3, 2017

I think this approach is OK as we can now manage tiles in a homogeneous way
I'm not concerned at this stage by performance overhead

@cdeil cdeil added the bug label Jun 28, 2018
@cdeil cdeil added this to the 0.3 milestone Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants