-
Notifications
You must be signed in to change notification settings - Fork 348
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
Ensure remote files have correct _g|p string in the name #3436
Conversation
This is the first thing we do in gmt_init_module as the file may be used for many things, such as finding -R.
I got a crash:
|
Yep, in grdimage. But the problem is in master too so I am working on a fix there. OK, perhaps wait to approve this branch until I can merge in from master later. |
Sorry, this is convoluted. Going to indexed images for the marbles was a good idea, but now we are taking into parts of @joa-quim code that has some issues, so it will be good to fix these. Here are some of the things I am fixing:
OK, fixing those and now it runs but the image is pure black. Strangely, I then found that the earth_day_01d_p.tif file was indeed black (!) while the others were fine (not sure why but will fix). So I then tried 30m which is a good image, but it also came out black from grdimage, so presumably the colormap is not set right or not used right. Hopefully have time after dinner... |
gdalinfo earth_day_30m.tif reports the color table, looks like this:
When I go through in debug in populate_metadata it find the same colors. Yet, the image that is produced looks like this: What is going on here, @joa-quim ? The command I run is
You can get the file via
PS: The 01d images that were black were produced by gdal_translate. Since they were small 360x180 I turned tiling off for those and then it worked. So a bug in GDAL somewhere. |
Note: To run this command without crashing you need to use the branch in #3438. |
Some progress. if I do not specify a projection then the image is correct. So the projection of the image is failing. |
OK, so I understand the problem now. GMT_Read_Image (via GDAL) finds an index image, and it just returns the index array and the colormap. It does not build an image. THen, when we need to project the image we are hopelessly interpolating a grid of indices... @joa-quim, does it ever make sense for us to return a non-image, i.e., a list of indices into a colormap? WOuld it not make more sense to expand that into a rgb image instead. At the very least we will need a index2rgb function that converts an index image to a rgb image and remotes the colormap stuff. I am guessing we do not have that? If we did this then all the stuff in grdimage that tries to do this on the fly when building the PS image would be simpler, plus it would actuallly work for this case. I am glad we decided to make the earth day/night images index so we actually found all this shit. Since you will be up hours before me, let me know what you think we should do. I think a gmt_index2rgb_image (GMT, struct GMT_IMAGE *I) would be helpful and it should be called when grdimage returns an indexed image. OR, we always do this. What good is it in grdmix and other modules to repeat this again and again. I do not think indices are helpful in our modules. |
I'll be out for the afternoon. |
I will make something similar to gmt_set_outgrid for images since if the input image is read only we cannot reallocate I->data. |
It does not make sense to convert an rgb image to indexed and project it. The result from the interpolation is garbage. But it does make sense to project a gray scale image. Think on the old satellite images that came in bands of 8 bits images. People reproject them. So at the end what make and does not make sense in on the users hand. |
Projecting color or gray is fine. projecting indices are not. That would only work if there is a one-to-one correspondence between the index 0-255 and the corresponding grayshade entry in the colormap (0-255).
written, compiles and is about to be debugged to see if I got it right. Returns true if we had to create a new image because input is read-only AND indexed. Otherwise we just reallocate internally and return I_in and false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Let's merge it in master and then test PR #3438.
This is the first thing we do in gmt_init_module as the file may be used for many things, such as finding -R. We are then guaranteed to have the correct name for a remote grid or image.
gmt grdimage -JG0/0/3i @earth_day_01d -B -pdf map
thus becomes internally
gmt grdimage -JG0/0/3i @earth_day_01d_p -B -pdf map
PS. This gets the image and takes me where I want but I think I have found a bug in grdimage so I will sort that out in a separate branch.