Add support for reading arcinfo adf tin layers #162 #164

vcloarec · 2019-10-11T02:43:11Z

First Draft, the mesh seems to be loaded. Need tests to verify if the geometry is ok.

saberraz · 2019-10-11T05:08:15Z

@vcloarec nice.
I tried it with the attached mesh.

mdalinfo tnz.adf
Mesh loaded: OK
  Driver: ESRI_TIN
  Vertex count: 13
  Face count: 19
  Projection: undefined
Datasets loaded: OK
  Groups count: 1
  Terrain Elevation

But when I try to open it in QGIS, I get core dump.
top.zip

PS: we can probably use the attached TIN for test and credit http://lab.usgin.org

PeterPetrik

very good start. just few small notes, but in general it looks very good.

PeterPetrik · 2019-10-11T06:02:23Z

mdal/frmts/mdal_esri_tin.cpp

+#include "mdal_esri_tin.hpp"
+
+//from https://stackoverflow.com/questions/41012414/convert-double-value-from-little-endian-to-big-endian
+template <typename T>


I think we have a function for this in https://github.com/lutraconsulting/MDAL/blob/master/mdal/frmts/mdal_selafin.cpp#L145

maybe we can just move that function to MDAL utils and use it here too?

I have added new function in MDAL utils, and I have changed selafin driver and esri tin driver.
Tell me what do you think about ?
But I am not sure that it is a good idea because the number of bytes read in the file depend now of the size of type. Is there a risk that this size of type is different from platform to other ? If yes, it is maybe not a good idea ...

PeterPetrik · 2019-10-11T06:04:01Z

mdal/frmts/mdal_esri_tin.cpp

+    if ( ! faceIn.is_open() )
+        return false;
+
+    //check if the file sizes of the nodes are coherent ?


i think it does not need to be... what you can check is first line of some file if they have some standard header/first line

There isn't more information that the position of the points and the indexes of the face. The unused first bytes (64 bytes for tnxy.adf, and 16 bytes for tnz.adf) are used by ArcGIS to store information about 4 virtual points created by ArcGIS for triangulation/generation (https://en.wikipedia.org/wiki/Esri_TIN, see Superpoint). Those points seem to be necessary for ArcGIS but not for MDAL.

So, for me, the only information to know if a file can be read is :

if it can be opened

if the size of the tns.adf file (Z value file) is coherent with the size of of tnxy.adf file.
I am not sure of the relevance of checking the size file coherence.

lets keep it as it is, just remove commented out code please

mdal/frmts/mdal_esri_tin.cpp

mdal/frmts/mdal_esri_tin.hpp

mdal/mdal_utils.cpp

PeterPetrik · 2019-10-11T06:10:21Z

mdal/mdal_utils.cpp

@@ -505,6 +506,12 @@ void MDAL::addBedElevationDatasetGroup( MDAL::Mesh *mesh, const Vertices &vertic
  mesh->datasetGroups.push_back( group );
 }

+void MDAL::addBedElevationDatasetGroup( MDAL::Mesh *mesh, const Vertices &vertices )


I would just use Bed elevation, not Terrain elevation . It is standard

For me "Bed Elevation" is specific for river bed elevation. Terrain Elevation could be more generic. As TIN are note only for river, that's why I purpose to keep Bed Elevation for format specific to hydraulic modeling, and maybe we can purpose another name for other format not specific to river.
A more generic name could be "Elevation" or simply "Altitude" ?

whatabout keeping the Bed elevation and just change the name in the TIN driver...?

tests/CMakeLists.txt

tests/test_esri_tin.cpp

nyalldawson · 2019-10-11T09:08:35Z

Wow, great work.... That was super fast!!

vcloarec · 2019-10-11T12:02:28Z

@saberraz
The issue is may be fix, it was bad indexes for faces
Edit :
Issue was not completely fixed. Now it is better, I can load mesh in QGIS.
Now, there are still work to do, like manage CRS, optimization and take account of comments of Peter!

vcloarec · 2019-10-11T12:05:05Z

@PeterPetrik , I try to look at all your comments may be later today.

saberraz · 2019-10-11T17:21:28Z

Nice, my test mesh works now!

vcloarec · 2019-10-11T19:05:51Z

Nice, my test mesh works now!

It is not the prettier TIN I've seen :) When it displayed on my screen, I believed there is a big issue in the code, but no...
The non-public TINs sent by Nyall look better!

nyalldawson · 2019-10-11T19:52:18Z

I've got some nice simple reference files incoming (freely licensed). Will post a link when I receive them!

vcloarec · 2019-10-12T00:45:47Z

I've got some nice simple reference files incoming (freely licensed). Will post a link when I receive them!

@nyalldawson
Is it possible to have TIN files with holes ?

nyalldawson · 2019-10-12T01:27:49Z

I'll ask. Also trying to get some with tagged edges/vertices...

vcloarec · 2019-10-14T02:52:52Z

Hello,
With this last commit, I think MDAL reads ESRI TIN as well as possible. Holes haven't been tested, but with coming Nyall files, maybe it will be done.
My comments on particular points below :

Superpoints :
If the first bytes of xy file and z file are usually used for particular points (called "superpoints" here) that are not used by MDAL, it seems it is not always the case. The file "thul.adf" contains the indexes of the superpoints that can be no the first. The code has been adapted to take into account superpoints that are not at the
beginning. But as we don't have a file with superpoints not at the beginning, it is not tested in such a situation.

Mask :
ArcGIS uses a mask to make invisible faces, those which are outside the outer boundaries, in holes or linked with superpoints. In this PR, MDAL simply used the mask file to ignore those faces.

Name of the TIN :
ESRI TINs are saved in a folder that the user gives when he creates it in ArcGis. All the different files in this folder always have the same names and have all the extension *.adf. As MDAL and QGIS used a filename to load a mesh, the trick is to retrieve the folder with any files with *.adf extension is provided to MDAL. So, in QGIS, selecting any *.adf files permits to load the TIN. The bad consequence is the name of the mesh is the name of the selected file, not the name of the folder...
I have implemented a method in the ESRI TIN driver to retrieve the name of the TIN. But now, it is not used because it seems not trivial for QGIS to obtain this name...

Endianness :
The endianness is handled by a new function in MDAL utils. This template function permits to read the value of any type in a binary file. An option provides the possibility to change the endianness of the value if needed.
Depending on the platform, if the type size is not compatible with the same type in the file, the caller keeps the responsibility to handle with this.
For Selafin driver, I have changed the read_double() method to see how this method could be improved. But, as some other methods could have to be changed too, I purpose to restore the initial code of this method. The Selafin driver could be improved in another commit/PR if the new template function is pertinent.

mdal/frmts/mdal_esri_tin.cpp

PeterPetrik · 2019-10-14T06:50:55Z

thanks a lot, merging!

nyalldawson · 2019-10-14T06:52:01Z

Awesome work -- this is very exciting!

@PeterPetrik any chance of a mdal version bump prior to QGIS 3.10.0?

nyalldawson · 2019-10-14T11:07:06Z

@vcloarec here's a zipped archive of a bunch of freely-usable tin reference files

tins.zip

It includes the original dems, the arcgis tins, and the results of exporting the tins to polygon layers (in the gdb file).

Hope it helps verify the holes/islands cases!

vcloarec · 2019-10-14T12:30:21Z

Thanks @nyalldawson , I quickly tried to load the mesh in QGIS, and all have been loaded, with holes and islands. I will add this TIN to the test folder and add tests, but later, I don't have time anymore...
I am very happy this enhancement seems to be in the next release of QGIS!

nyalldawson · 2019-10-17T04:39:24Z

@vcloarec

For completeness, the attached updates to the tins adds an example with tagged vertices. That should cover all the functionality possible in this format:

dem (2).zip

PeterPetrik · 2019-10-17T06:57:24Z

@nyalldawson @vcloarec any possibility you have time to add those to test suite?

vcloarec · 2019-10-17T12:35:36Z

@nyalldawson @vcloarec any possibility you have time to add those to test suite?

I don't have a lot of time this week for that. I will try next week.

vcloarec · 2019-10-25T04:05:03Z

Hi,
I started to add the test with the TINs give by @nyalldawson.
First test, no problem, the vertices count and the faces count read by MDAL are the same than in the gdb files.
When testing the dem with hole, there is a little difference with vertices count between MDAL and the gdb file. Two of those extra vertices are in blue in the picture below (the underlayer is the raster TIN, the source ?)

I think, during any of the conversions, faces have gone away and the vertices stay alone... Maybe, this is due of the border of the raster TIN.

So, in the ESRI TIN files, it seems there are vertices without faces ...
I purpose to implement a process to remove those isolated vertices, but it will slow down the loading...
What do you think about ?

nyalldawson · 2019-10-25T05:22:16Z

@vcloarec I can't work out quite where in the tin you are looking -- can you show the whole extent?

nyalldawson · 2019-10-25T05:23:31Z

@vcloarec disregard that, sorry!

nyalldawson · 2019-10-25T05:27:00Z

@vcloarec
Here's what I see in arcmap:

So those extra vertices are attached only to edges with a type of "Outside edge" (3). I gather you are currently stripping these away during the conversion?

vcloarec · 2019-10-25T12:29:45Z

@nyalldawson, yes, outside faces (so implicitly associated edges, as MDAL doesn't store directly the edges) are stripped away.
For my information, what is the relation between the raster DEM (tif) and the TIN ? If the raster DEM is a conversion from the TIN, it is weird that some pixels are outside the boundary :

In any cases, even if there is no visual consequences to keep the isolated vertices, I think it is cleaner to strip away this vertices too when loading the mesh.

vcloarec force-pushed the master branch from 2470dda to 7c4d864 Compare October 11, 2019 03:24

PeterPetrik self-requested a review October 11, 2019 05:58

PeterPetrik requested changes Oct 11, 2019

View reviewed changes

PeterPetrik added 2D enhancement New feature or request formats formats/providers labels Oct 11, 2019

PeterPetrik added this to the MDAL 0.4.0 milestone Oct 11, 2019

vcloarec force-pushed the master branch from f4cda2d to 862f136 Compare October 14, 2019 02:19

vcloarec changed the title ~~[WIP] Add support for reading arcinfo adf tin layers #162~~ Add support for reading arcinfo adf tin layers #162 Oct 14, 2019

Add support for reading arcinfo adf tin layers lutraconsulting#162

2b89913

vcloarec force-pushed the master branch from a604191 to 2b89913 Compare October 14, 2019 03:26

PeterPetrik requested changes Oct 14, 2019

View reviewed changes

mdal/frmts/mdal_esri_tin.cpp Outdated Show resolved Hide resolved

Comments for unknown crs

9b8f676

PeterPetrik merged commit 6f9ebab into lutraconsulting:master Oct 14, 2019

saberraz mentioned this pull request Oct 14, 2019

Support for 3d visualisation of ESRI TINs qgis/QGIS#32240

Closed

vcloarec mentioned this pull request Oct 26, 2019

Tests for ESRI TIN driver. Change approach for loading the TIN #172

Merged

joewheaton mentioned this pull request Aug 6, 2022

Implement TIN Rendering type Riverscapes/QRAVEPlugin#73

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for reading arcinfo adf tin layers #162 #164

Add support for reading arcinfo adf tin layers #162 #164

vcloarec commented Oct 11, 2019

saberraz commented Oct 11, 2019

PeterPetrik left a comment

PeterPetrik Oct 11, 2019

vcloarec Oct 11, 2019

PeterPetrik Oct 11, 2019

vcloarec Oct 11, 2019

PeterPetrik Oct 11, 2019

PeterPetrik Oct 11, 2019

vcloarec Oct 11, 2019 •

edited

Loading

PeterPetrik Oct 11, 2019

nyalldawson commented Oct 11, 2019

vcloarec commented Oct 11, 2019 •

edited

Loading

vcloarec commented Oct 11, 2019

saberraz commented Oct 11, 2019

vcloarec commented Oct 11, 2019 •

edited

Loading

nyalldawson commented Oct 11, 2019

vcloarec commented Oct 12, 2019 •

edited

Loading

nyalldawson commented Oct 12, 2019

vcloarec commented Oct 14, 2019 •

edited

Loading

PeterPetrik commented Oct 14, 2019

nyalldawson commented Oct 14, 2019

nyalldawson commented Oct 14, 2019

vcloarec commented Oct 14, 2019 •

edited

Loading

nyalldawson commented Oct 17, 2019

PeterPetrik commented Oct 17, 2019

vcloarec commented Oct 17, 2019

vcloarec commented Oct 25, 2019

nyalldawson commented Oct 25, 2019

nyalldawson commented Oct 25, 2019

nyalldawson commented Oct 25, 2019

vcloarec commented Oct 25, 2019

Add support for reading arcinfo adf tin layers #162 #164

Add support for reading arcinfo adf tin layers #162 #164

Conversation

vcloarec commented Oct 11, 2019

saberraz commented Oct 11, 2019

PeterPetrik left a comment

Choose a reason for hiding this comment

PeterPetrik Oct 11, 2019

Choose a reason for hiding this comment

vcloarec Oct 11, 2019

Choose a reason for hiding this comment

PeterPetrik Oct 11, 2019

Choose a reason for hiding this comment

vcloarec Oct 11, 2019

Choose a reason for hiding this comment

PeterPetrik Oct 11, 2019

Choose a reason for hiding this comment

PeterPetrik Oct 11, 2019

Choose a reason for hiding this comment

vcloarec Oct 11, 2019 • edited Loading

Choose a reason for hiding this comment

PeterPetrik Oct 11, 2019

Choose a reason for hiding this comment

nyalldawson commented Oct 11, 2019

vcloarec commented Oct 11, 2019 • edited Loading

vcloarec commented Oct 11, 2019

saberraz commented Oct 11, 2019

vcloarec commented Oct 11, 2019 • edited Loading

nyalldawson commented Oct 11, 2019

vcloarec commented Oct 12, 2019 • edited Loading

nyalldawson commented Oct 12, 2019

vcloarec commented Oct 14, 2019 • edited Loading

PeterPetrik commented Oct 14, 2019

nyalldawson commented Oct 14, 2019

nyalldawson commented Oct 14, 2019

vcloarec commented Oct 14, 2019 • edited Loading

nyalldawson commented Oct 17, 2019

PeterPetrik commented Oct 17, 2019

vcloarec commented Oct 17, 2019

vcloarec commented Oct 25, 2019

nyalldawson commented Oct 25, 2019

nyalldawson commented Oct 25, 2019

nyalldawson commented Oct 25, 2019

vcloarec commented Oct 25, 2019

vcloarec Oct 11, 2019 •

edited

Loading

vcloarec commented Oct 11, 2019 •

edited

Loading

vcloarec commented Oct 11, 2019 •

edited

Loading

vcloarec commented Oct 12, 2019 •

edited

Loading

vcloarec commented Oct 14, 2019 •

edited

Loading

vcloarec commented Oct 14, 2019 •

edited

Loading