hipspy / hips

Python library to handle HiPS
https://hips.readthedocs.io
12 stars 16 forks source link

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

Open adl1995 opened 7 years ago

adl1995 commented 7 years ago

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?

tboch commented 7 years ago

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

tboch commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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

tboch commented 7 years ago

@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 commented 7 years ago

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

cdeil commented 7 years ago

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 commented 7 years ago

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