arsf / lag Goto Github PK

Errors such as file I/O failing currently are not descriptive about the kind of problem that happens. Operations like saving files to a directory the user cannot access aren't obvious to the user about why they failed. Message windows should appear with more helpful text when things like this does happen.

Places to check:

• LAS file opening and loading
• Temporary file caching
• LAS file saving

What is the simplest data format that LAG supports? By "simple" I mean something that can be programmatically generated in a few lines of code from data in an array, without dragging the entirety of liblas into the picture.

EDIT: due to a distro quirk, I cannot have lastools and LAG installed simultaneously. Therefore conversion tools offered by the lastools package are not available to me.

So, I just moved some las points by about 15 metres to test something else, but to check it worked correctly, I confirmed with the ruler that it was about the right distance. It struck me how hard it is to actually make accurate readings with the ruler.

If I hadn't of known exactly how far I'd moved the las points, I would have had an error of about +/- 10% in my measurement of a really clearly defined and obvious object.

The ruler could do with a couple of aides, like vertical and horizontal lines crossing the screen at each end of the ruler, and a breakdown of vertical and horizontal components of each measurement. I would also be interested in finding ways to measure or estimate differences between not-so-clearly defined features. Maybe something like drawing two circles to get the maximum, minimum and likely ranges of values that could indicate the distance between two clouds.

Ensure LAG is capable of supporting the LAZ file format. The laslib library supports various, but transparent, improvements, which take effect when using LAZ files. For example, Martin Isenburg suggests using the laslib provided clipping tests in LoadWorker.cpp at approx line 304, replacing

if (usearea)
{
    if (!vectorTest(fence.getXs(), fence.getYs(), 4,
        reader->point.get_x(), reader->point.get_y()))
    {
        skip_counter = 0;
        continue;
    }
}

with something like

reader->inside_rectangle(fence.getXs()[0], fence.getYs()[0], fence.getXs()[1], fence.getYs()[1]);

I would like for LAG to have programmable colour responses in every LagDisplay. That means, colour for each point is computed based on that point according to some user-defined function. Then, the "Colour By" menu would consist of a list of default presets, and an item to open the user-defined colour-response dialogue.

This would allow for unusual use-cases, since things like intensity can vary wildly between data from different sensors or processors. Such cases could also involve highlighting points of interest, without having to classify them in another program first, such as "colour points above this height with at least this intensity as this bright colour".

A simple implementation would store a matrix, which when applied to a lidar point would produce a colour. Better though, would be a programmable interface with things like if conditions, although the impact on efficiency would go from bad to severe.

This improvement may have to wait until the rendering caches in a smarter way, since it complicates the colouring process for every point by quite a lot.

When the window is too narrow, widgets start disappearing. I did expand the default size of the window at one point, but it still wasn't enough, and regardless, it's not a good solution for medium to small monitors.

Work out how to shrink the size of the icons across the top of each window to make room.

The code for the lidar quadtree looks like some improvements could be made in terms of what needs memory allocated to it, what can be derived, and what can be shared. The smaller these are in memory, the faster they can be allocated and deallocated.

The immediate improvement I can think of would be...

• Making a union between bucket_ and the four child nodes.
• Defining a quadtee metadata structure and holding pointers to that, rather than a string, pointer to cache minder, resolution base, etc.

Program behaviour is reported to change depending on the version of OpenGL available. Older versions of OpenGL reportedly cause the program to core dump. Should ideally print out a polite error detailing the problem before dying.

If the OpenGL version affects the stability of the program, this should be checked for the windows port too, as mingw uses a different instance of OpenGL.

After loading a LAS file, if the resolution base is then changed and a new LAS file is loaded, the program segfaults. The following backtrace reports the problem to be in the PointBucket header.

#0  0x000000000045a2bb in PointBucket::getNumberOfPoints (this=0x7fff5e3dcac0, resolution=0)
    at /users/rsg/besm/.build/include/lidar/quadtree/PointBucket.h:155
#1  0x000000000045f33e in TwoDeeOverview::drawpointsfrombuckets (this=0x7ffff5b90a80, buckets=0x7fffe03f0000, numbuckets=1214, drawnbucketsarray=
    0x7ffff589d200, cachedonly=true) at src/TwoDeeOverview.cpp:509
#2  0x000000000045f109 in TwoDeeOverview::mainimage (this=0x7ffff5b90a80, buckets=0x7fffe03f0000, numbuckets=1214) at src/TwoDeeOverview.cpp:413
#3  0x00000000004c0981 in DrawWorker::run (this=0x7fffe1429800) at src/DrawWorker.cpp:121
#4  0x0000000000458a5c in sigc::bound_mem_functor0<void, Worker>::operator() (this=0x7ffff5a14958) at /usr/include/sigc++-2.0/sigc++/functors/mem_fun.h:1787
#5  0x00000000004583b0 in sigc::adaptor_functor<sigc::bound_mem_functor0<void, Worker> >::operator() (this=0x7ffff5a14950)
    at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:251
#6  0x0000000000457997 in sigc::internal::slot_call0<sigc::bound_mem_functor0<void, Worker>, void>::call_it (rep=0x7ffff5a14920)
    at /usr/include/sigc++-2.0/sigc++/functors/slot.h:103
#7  0x0000003230a428dd in ?? () from /lib64/libglibmm-2.4.so.1
#8  0x000000307b46a305 in ?? () from /lib64/libglib-2.0.so.0
#9  0x0000003079c07d14 in start_thread () from /lib64/libpthread.so.0
#10 0x00000030798f167d in clone () from /lib64/libc.so.6

Lag has a chance of crashing with every file it loads, which makes loading very large datasets of around 20 files very difficult. This chance is very roughly guesstimated to be around 1 in 20. The problem likely lies in the LoadWorker thread.

The latest version on my winport branch (13 August onwards) removes the line

fs::create_directory(cacheFolder / fs::unique_path("lag_%%%%-%%%%"));

from Quadtree.cpp line 93, as it was causing problems in windows runtime, and it seemed to be that the created directories were unused; that lag was creating these directories and subsequently ignoring them. It's uncertain how long this code was dead for.

This needs to be reimplemented, and lag made to use its respective temporary folder and delete it appropriately on closure of the program.

Determine whether LAG is making use of files exclusive to the RSG system, which would prevent any third parties from easily compiling and installing LAG. If that is the case, request these files upstream and/or provisionally provide them from LAG.

Principally, headers, library objects, and .pc files from laslib need to be checked. Also, any include statement that refers to anything like <laslib/someheader.hpp> may need to be changed to just <someheader.hpp>.

The "Heights" button for the profile often seems to report wildly varying results, often highly contrary to what seems to be visibly obvious. Check the get average heights method in Profile.cpp.

Currently, all thread management of the lidarquadtree in the program is regulated by a universal global_pointbucket_mutex object, which is held by any agent performing any operation with any pointbucket. To add clarity to the structure of the program, it would be better to consolidate the responsibility of managing quadtree threading into one place.

Sometimes, somehow, the profile get's stuck on one particular frame. Refreshing the frame by moving around or zooming though, reveals the frame the user would expect before the program flickers back to the frame it's stuck on.

• It's not clear how this is reproduced, may have something to do with using the classification queue.
• The program cannot get a new profile and display it, even by moving the viewpoint around, in this state.

When loading new las files into an instance of LAG which has already got a loaded quadtree, the subsequent deletion of said quadtree has a chance of causing a segfault. Estimated, 5-30% chance, give or take.

Quadtree closure at program termination in the windows port causes a segfault to occur. The problem can be traced to boost::filesystem::path's destructor, which can only come from the instanceDirectory_ variable in Quadtree.

Normally, C++ only fails to destroy objects within other objects when the parent object is not instantiated at the time of deletion. Either two deletions are occurring or Quadtree is failing to instantiate earlier in the program.

This may be symptomatic of the underlying problem with rendering in the present version of the windows port.

Sometimes, telling LAG to get a profile causes a segfault. This rarely seems to happen when LAG has recently been loaded. So, at a guess, at least 5 profiles need to be taken before there is a risk of this.

Could still be a race-condition. Alternatively I would look at where the memory for the profile is allocated and deallocated.

Point highlighting was removed in the late threading improvement commit. Re-implement it, preferably in a way that's useful to use where the point size is 1.

After file loading, the user is presented with nice black TwoDeeOverview and Profile windows. The icons that were previously present are blacked out but may become visible with mousing over, and appear to even be usable. Despite the total lack of usability, the program seems content to run like this indefinately.

As the state of the rendering process is invisible, gdb needs to be used to determine how much of the program is and is not running.

Some problems seem to be difficult to reproduce between different users and systems. LAG probably needs a mechanism for logging events and problems for post-mortem analysis.

• Trivial events need to be logged on a regular basis. The system needs to either be efficient, or have the option to be enabled/disabled, perhaps with a command-line argument.
• The system needs to be threadsafe. Time constraints mean per-use-case thread management is not an option.
• The logging system cannot reflect the functionality that usleep served previously, that is, the system must behave the same with logging as it does without logging. If a slowdown occurs, many race-conditions could easily be suppressed just by enabling logging.

The mixture of tabs and spaces is bad. Change all tabs to spaces. Convention is 3 spaces to 1 tab.

ASCII files don't load. Either they didn't ever load, or some recent change has broken that functionality.

I think they never worked, because it looks like LoadWorker has, for a very long time, worked on the following code:

    int points_to_load = reader->header.number_of_point_records
          / (point_offset + 1);

    while (!stopped && points_loaded < points_to_load
          && reader->read_point())

For ASCII files, reader->header.number_of_point_records always has been 0, because ASCII files contain no convenient headers from which to extract that information from. And so, the while clause will always evaluate as false on the first iteration, meaning no points will ever have been able to load while those two lines have been in place. (Unless, of course, the behaviour of laslib has changed. Unconfirmable though, as laslib has no source code management at all.)

If the fence is changed while classifying points in the profile view, the classification may (only?) happen for points inside the fence after the fence has been moved. Users expect only the points inside the fence when the classify button was clicked to be classified.

Opening files is a pain when there is no sane default for the filechooser dialog, and the files desired are at the end of a very long filepath. A better received default would be to open the filechooserdialog at the current directory. This could save noticeable amounts of time and effort.

It would also be nice if files to be loaded could be specified on the command line with globbing, but that could come later.

When dragging the image in the viewport too quickly, the image seems to momentarily flicker in places of the screen that it shouldn't ever need to pass through. May be worked around by dragging slowly.

Leaving a ruler drawn in the profile view crashes LAG. This is unconfirmed, and may apply also to the main window.

Making some unrelated changes I noticed that lidarquadtree (and maybe the main body of LAG) mostly just defines every value and function as protected, then makes the classes that need those classes friends.

I poked around looking at QuadtreeNode as my example. I'm certain that this class is completely internal to the quadtree. QuadtreeNode is not an outward facing class, nothing outside of the library should be able to access it or refer to it, so I would argue that everything that uses it should use only public components (of which there are currently none), and that QuadtreeNode should have no friends at all.

What I would really like is for C++ to be more capable of specifying more exactly what should be able to access what. Or perhaps these classes should be more broken up? Just speculation.

It seems that the original threading architecture within lag was intended to be capable of indicating to the drawing thread that it should terminate if a call to drawviewable occured before it was naturally able to do so. I doubt, from the implementation, that this ever worked originally.

The idea is good in concept and ought to be implemented using the new mutex and condition system, in the TwoDeeOverview::drawviewable method. An attempt was previously made, as follows, but causes rendering to fail completely for an unknown reason.

if (drawing_thread->isDrawing())
{
   abortFrame(true);

   // functions that were tried that did not fix the problem
   //awaitClearGLControl(true);
   //signal_FlushGLToScreen();
}

This may be that, first of all, the drawing thread does what it can for every drawviewable call, explaining the occasional artifacts that do render. And secondly, the final call to drawviewable in any short period of time leaves the drawing thread in a state where it thinks it is not supposed to render.