openjump-gis / openjump

OpenJUMP, the Open Source GIS with more than one trick in its kangaroo pocket, takes the leap from svn to git. join the effort!
http://openjump.org
GNU General Public License v2.0
28 stars 14 forks source link

Single Use > Raster Styles classification doesn't work #112

Closed robertomariarossi closed 1 week ago

robertomariarossi commented 1 month ago

When Trying to classify a discrete Raster using Raster Styles > Single values, the value represented are outside the real range of the raster values. In the example attached (I attach the raster and the screenshots) a Raster file with a 0-100 range, when loaded, is correctly represented. When trying to use a Single values styles the values which appear in the legend do not correspond to reality

Roberto Rossi

CN.zip

Screenshot 2024-05-24 171729 Screenshot 2024-05-24 171952 Screenshot 2024-05-24 172009 Screenshot 2024-05-24 172120

robertomariarossi commented 1 month ago

version 20240516, release 5251, used

jratike80 commented 1 month ago

I think that the more critical bug is that for some reason values 15.0 and 30.0 are not sorted correctly. Letting a software to create classes for all the unique values in a 32 bit data feels dangerous, what if the whole data range is really in use with full precision? But perhaps OJ does not try to do that.

I tried "Single values" with a 32-bit DEM file with different data range. It looks like the "Single values" are not unique values in the data but classified into full meters. And also with this file some classes are sorted into wrong order: 2.0 - 0.0 - 3.0 - 4.0....

image

Test data can be found here https://www.nic.funet.fi/index/geodata/mml/dem2m/2008_latest/P3/P34/. Maybe with these images the sorting order issue from "Ramp all" appears always with the last entry. For example P3411F:

image
edeso commented 1 month ago

can you provide a sample file and a screenshot for illustration?

robertomariarossi commented 1 month ago

can you provide a sample file and a screenshot for illustration?

I already attached them in the original post (or were you referring to jratike80? :-) )...

Rob

ma15569 commented 1 month ago

Possibly the problem is how the Single Style framework collects values from a raster and sort them. I gave a look into the corresponding class (org.openjump.core.rasterimage.styler.ui.SingleValuesPanel) and checked that the method findUniqueValues uses a TreeSet class. But Treeset sort the number as string in lexicographical order. Not sure is the best way to order number which sometimes have also decimals

edeso commented 1 month ago

But Treeset sort the number as string in lexicographical order. Not sure is the best way to order number which sometimes have also decimals

quickly debugged it and found that the sorting was not actually applied. that is fixed in the last commit. natural order sorting should take decimals into account if correctly implemented.

as to the range provided i will have a look.

edeso commented 1 month ago

hmm

https://github.com/openjump-gis/openjump/blob/main/src/org/openjump/core/rasterimage/styler/Utils.java#L16-L54

returns the range from 0 - 255 as if it were in the data. are we sure that min 25 and max 92 values are correct? seems like as they are retrieved via rasterImageLayer.getMetadata().getStats() in https://github.com/openjump-gis/openjump/blob/main/src/org/openjump/core/rasterimage/styler/ui/RasterStylesDialog.java#L374-L375

edeso commented 1 month ago

what i can see is that the data is read neither as Float or Double but simply as Integer because the datatype is declared as DataBuffer.TYPE_USHORT

https://github.com/openjump-gis/openjump/blob/main/src/org/openjump/core/rasterimage/styler/Utils.java#L26-L32

edeso commented 1 month ago

ideas? not familiar with that code at all ;)

ma15569 commented 4 weeks ago

Hi Ede, I apologize for the late answer. On these days I am busy with the final tests at School. I understand a bit that code. From next Monday I am a bit free and I can give a look at it Peppe

Il mar 28 mag 2024, 20:26 edeso @.***> ha scritto:

ideas? not familiar with that code at all ;)

— Reply to this email directly, view it on GitHub https://github.com/openjump-gis/openjump/issues/112#issuecomment-2135868047, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSYPMZSC4LL5N7552T7BY3ZETD4XAVCNFSM6AAAAABIHZJAKWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZVHA3DQMBUG4 . You are receiving this because you commented.Message ID: @.***>

edeso commented 3 weeks ago

np Peppe, please do. find my observations above. why the initial range of 25-92 results in 0-255 leaves me a bit stumped.

ma15569 commented 2 weeks ago

Hi Ede, please check my last two commits. They should solve the bug found by Roberto Peppe

ma15569 commented 2 weeks ago

A) class org.openjump.core.rasterimage.styler.ui.IntervalPanel.java line: 293: double[] rasterData = Utils.purgeNoData(rasterImageLayer.getActualRasterData(),rasterImageLayer);

Should be set to: double[] rasterData = Utils.purgeNoData(rasterImageLayer.get.getRasterData(null),rasterImageLayer);

B) rasterImageLayer.getActualRasterData() had probabily a reason to limit the values to 225: The presence of more values freeze OpenJUMP when I try to style a rasterImageLayer

ma15569 commented 2 weeks ago

Going back to the poin B of previous post. 1) the raster CN.tif that Roberto added to this issue represents a variable info defined by few classes of values, only 27, with a range of value between 25 to 92. This is called sometimes a categorical raster. This file defines a variable in hydrology called curve number (https://en.wikipedia.org/wiki/Runoff_curve_number) and the raster data of the file is usually is an Integer or byte. In this case it is usefull to define a Single Style Framework where each class is defined by a color (or symbol)

2) there are other raster that can express a continuous range of values, Cost accumulation or distance for instance, The continuos range of value is apparent, it can be at least the same number of the cell of the raster. But pratically it can be a very large number that Openjump Raster style requires a large ammount of RAM (and time). Nevertheless there is non useful need to express the style of these rasters with a Single Style Framework: the Interval Style or the Continuous one (the other two panels in Raster Style) do the job.

To avoid that non expert user uses Single Style Framework for continuous raster we (and freeze OJ) I will add a warning message into the SingleValuesPanel class, method formComponentShown , that advice the user and stop to procede the styling operation

ma15569 commented 2 weeks ago

AccCost.zip I added an exaple of continuous raster. It is so called Cost accumulation (created with my Raster tools). There are 246410 cells and 2417766 classes of values

edeso commented 1 week ago

To avoid that non expert user uses Single Style Framework for continuous raster we (and freeze OJ) I will add a warning message into the SingleValuesPanel class, method formComponentShown , that advice the user and stop to procede the styling operation

how about we define a practical limit like 256 and simply disable Single Style creation for those? the warning can still be shown unobtrusively in the tab instead of the usual set up components.

edeso commented 1 week ago

https://github.com/openjump-gis/openjump/pull/117 works fine. will commit manually though as line endings are modified all over the file (again ;()

just curious. what is the difference between getActualRasterData() and getRasterData(null). looked at the code and see the latter is using the displayed image data, but how does that differ from the source data?

edeso commented 1 week ago

@robertomariarossi can you please confirm that this is solved in the latest snapshot? THX!

ma15569 commented 1 week ago

@Ede: just curious. what is the difference between getActualRasterData() and getRasterData(null). looked at the code and see the latter is using the displayed image data, but how does that differ from the source data?

No idea. It seems something left behind. Possibly the author wanted to use it as a limit size of used data (the total range of raster data divided 256= classes) but it was left uncorrect (total range of raster data=256=classes)

@Ede how about we define a practical limit like 256 and simply disable Single Style creation for those? the warning can still be shown unobtrusively in the tab instead of the usual set up components.

I talked with Roberto. We think to make two steps of warning

a) when the classes of data are more than 512 (and less than 2042): a warning advising the user that the classes are many for a single style and possibly there could happen that rendering time slows down. User can go on or can use a different style. See the screenshot below. [image: image.png]

b) when the data are more than 2048, the Single style panel is disable with a warning message. See the screenshot: [image: image.png]

Il giorno sab 22 giu 2024 alle ore 15:33 edeso @.***> ha scritto:

@robertomariarossi https://github.com/robertomariarossi can you please confirm that this is solved in the latest snapshot? THX!

— Reply to this email directly, view it on GitHub https://github.com/openjump-gis/openjump/issues/112#issuecomment-2184038145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQSYPM5ZZXVGX2QD43VXTJLZIV4KHAVCNFSM6AAAAABIHZJAKWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBUGAZTQMJUGU . You are receiving this because you commented.Message ID: @.***>

robertomariarossi commented 1 week ago

@robertomariarossi can you please confirm that this is solved in the latest snapshot? THX!

I checked the 5256 release with some categorical (discrete) raster, and everything works fine! Thank you Giuseppe!

In my opinion it should be better to introduce a warning to avoid inexpert users to apply Single values to continuos raster as we discussed with Giuseppe.

Rob