biglimp / qgis-fusion

QGIS3 plugin for FUSION/LDV
GNU General Public License v3.0
7 stars 0 forks source link

[FUSION/LDV] 64 bit architecture in QGIS #3

Closed spono closed 4 years ago

spono commented 4 years ago

Hi, according to the release 4.x of FUSION, the tools are being translated to the 64bit architecture. Do you have any plan to allow your plugin to take advantage of it?

I guess an easy way could be to include a checkbox in the plugin settings (e.g. "use 64bit version") and add an if clause in the executable selection, like:

    def processAlgorithm(self, parameters, context, feedback):
        if checkbox64:
            commands = [os.path.join(fusionUtils.fusionDirectory(), 'PolyClipData64.exe')]
        else
            commands = [os.path.join(fusionUtils.fusionDirectory(), 'PolyClipData.exe')]

Unfortunately I'm just an end user and I can't help with scripting.

biglimp commented 4 years ago

I didnt see the new version of FUISON. Yes, there should definitely be a plan to make use of the 64-bit algorithms. I will put it up on the todo list...

biglimp commented 4 years ago

Actually, I just tried with gridmetrix (recent commit). Please try and report back if it works on your end. If so, I can start adding it to other algorithms as well.

spono commented 4 years ago

I just gave it a try and it seems to work: thanks!

a couple of things:

  1. when the explorer opens up to select the file, it doesn't show me any .laz file (only las, but there's no other option)....do you know to what it may be related to? here a screenshot:

missing_las

  1. just a couple of thoughts about "Version64": personally, I would keep the checkbox turned on or maybe move it to the settings panel (where the FUSION path is set) to allow to set only once the choice for all the tools. On the other hand, there might be issues with those not traslated to 64 yet. Do you think another option could it be to set a sort of:
exe.list = list of exe files in FUSIONpath

if "ToolName64 matches a value in exe.list" or "version64"
    use ToolName64
else
    use ToolName

I guess it would be flexible enough without the need to specify which version to use...or I'm missing something?

biglimp commented 4 years ago
  1. Ok, I will fix so that laz files can be loaded
  2. I think I will add 64 bit separately and then keep the option activated where possible.
spono commented 4 years ago

ok, now it is able to see las & laz and it can call 64-bit exe. I'll help you out with adding the call to 64bit in the other tools and then push a PR.

Before that, I have to ask you to [please] check #5 and then I'll proceed!

biglimp commented 4 years ago

I merged your contributions. Just some minor spelling things that I corrected. Thanks you for your contribution! Do you want to be added as an author of this plugin (your name (Niccolo' Marchi) will be added to the metadata file and visible in the QGIS plugin repository)?

spono commented 4 years ago

oh, well...ok, thanks :)

BTW, I'm gonna push another PR in the next hour and then I think I'll stop for a while (finished my free time)....so, if you want, I believe a new version of the plugin can be safely released.

One thing: I'm trying to update my old scripts according to what you and Alex wrote (I'm not a Pythonist...I can read it and only make copy-paste-wise modifications). Unfortunately, there's one thing I don't understand of the logic related to the input of multiple files: there should be the way to create a .txt file with a list in case more than one is selected...but I haven't been able to make it work. You can find an example in xyz2dtm :

        self.addParameter(QgsProcessingParameterMultipleLayers(self.INPUT,
                                                               self.tr('XYZ files'),
                                                               QgsProcessing.TypeFile))

and then calling:

        fileList = fusionUtils.layersToFile('xyzDataFiles.txt', self, parameters, self.INPUT, context)
        arguments.append(fileList)

That could be quite a big thing for users, but I don't know how to fix it...

biglimp commented 4 years ago

Great. I wait for one more commits before I update the QGIS plugin repo.

Make sure that you pull the new code that I just submitted before you add a new tool.

We can set up multiple files in the future, on a todo list...

biglimp commented 4 years ago

This issue with QgsProcessingParameterMultipleLayers is that it is designed for map layers and not regular files (I think).

spono commented 4 years ago

mmm...ok

Another thing: there are some tools (e.g. TreeSeg) that require just a general “file name” that will be applied to many different outputs. If I set just a simple name (e.g. "test_output") it returns an error such as “invalid name”; to fix this the user has to enter "test_output.csv" (or any other extension) and FUSION strips it out. Do you know how to make it work without such error?

biglimp commented 4 years ago

Maybe you can use QgsProcessingOutputFolder to locate the output directory of the files and the QgsProcessingOutputString to add a base filename...

spono commented 4 years ago

it returns:

File "C:/Users/NM/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\processing_fusion\algs\densitymetrics.py", line 106, in initAlgorithm
              self.tr('Base name for output files')))
             TypeError: QgsProcessingAlgorithm.addParameter(): argument 1 has unexpected type 'QgsProcessingOutputString'
biglimp commented 4 years ago

hm. Mayby you should use QgsProcessingParameterString instead. I am not sure...

biglimp commented 4 years ago

I close this now and we can continue on another issue...