Esri / military-tools-geoprocessing-toolbox

military-tools-geoprocessing-toolbox is a collection of models, scripts, and tools for use in ArcGIS for Desktop and ArcGIS Pro. This toolbox is one component that is a part of Military Tools.
Apache License 2.0
33 stars 14 forks source link

Linear Line of Sight symbology incorrect in ArcGIS Pro #387

Closed dfoll closed 5 years ago

dfoll commented 5 years ago

Linear Line of Sight has an output layer for Sight Lines which is a binary visible/non-visible - white/black output, in ArcGIS Pro they are backwards, when there is a sight line between observer and target they are being colored black and when there is no direct sight line, they are being colored white.

Output target features are symbolized red or green for non-visible or visible and labeled with how many observers can see them.

Expected Behavior

image Sight lines to visible targets are white, sight lines to non-visible targets are black. Targets are colored green and labeled with number of observers that can see them or red if they are non-visible. This screenshot is taken from ArcMap to show correct symbology

Current Behavior

image Sight lines are reversed, the visible ones are black. Targets are all red, instead of being shown as green for ones that are visible (it is currently known that in ArcGIS Pro we are having trouble with the labels).

Steps to Reproduce (for bugs)

  1. Run Linear Line of Sight GP tool from the MT build 126 GP toolbox
  2. Observers: LLOS_Observers
  3. Targets: LLOS_Targets
  4. Input Elevation Surface: ElevationUTM_Zone10
  5. Keep all other defaults Test data can be found in MilitaryToolsTestData.gdb

Your Environment

Was able to reproduce with ArcGIS Pro 2.2 with September release and build 126, and ArcGIS Pro 2.3 with build 126

BobBooth commented 5 years ago

This is what I'm currently seeing, using the GP tool in the Military Tools.pyt image.png The black and white lines are correct, sorry. But the red Target circle should be green if the three lines from the observers that can see it are white (target is visible). image.png

BobBooth commented 5 years ago

It looks as though the problem is that particular point is visible from three of the four observer locations, but it is not visible from the last of the four, and that feature is the one being drawn on top, so it shows as red. image.png Perhaps for Pro we need the OutputTargets layer to be symbolized using the Frequency attribute. The values are either NULL (for not visible from anywhere) or a number representing the number of observers who can see the location. We could symbolize this with a Unique Values renderer on the Frequency field, where the value Null gets a red symbol, and all other values get a green symbol. image.png

The Null value might be a problem, for output feature class containers that for some reason do not support null values. We may need to update the tool to include a "visible at all" output field that is calculated to 1 if the frequency is greater than 1 and to "0" if it is null or 0, and use that.

csmoore commented 5 years ago

It looks to me like Pro and ArcMap are interpreting the symbol layer drawing order differently (the tool using the same .lyr file for both), just switching the symbol layer orders in Pro seems to resolve the issue.

To resolve we will probably have to have a different lyrx file for Pro (versus reusing the ArcMap one like we have been doing).


Update: turns out there was already a lyrx file in the repo that seems to symbolize correctly. Minor update to code made in https://github.com/Esri/military-tools-geoprocessing-toolbox/commit/a1e9e2f180b78d74a0011c491f2c493e5e5f8533

Should look like ArcMap with change: image

csmoore commented 5 years ago

While testing the target symbology on 2.2 (sightline issue doesn't show up on 2.3), I also see original issue with the reversed sightlines. Looks like this was also a Pro bug with how it interpreted the ArcMap .lyr file, but this can also be fixed/worked-around by using the Pro lyrx file from the repo.

These output symbology bugs are likely left over from when we combined the Pro and ArcMap toolboxes.

csmoore commented 5 years ago

Addressed in PR #397 - ArcMap version of lyr files where not displaying correctly/consistently on Pro so switched tool to use the Pro versions (lyrx) that were already available in the repo.

Should match ArcMap output now:

image

BobBooth commented 5 years ago

I pulled from @csmoore branch and tested, symbology looks correct, but layer descriptions are 0 or 1, should be verbose/human readable "Visible" "Not Visible" etc... I will update the LPKXs. image

csmoore commented 5 years ago

Latest dev seems to look fine for me on 2.3 so not quite sure what the issue might be:

image

csmoore commented 5 years ago

Running on Pro 2.1, I saw the same problem @BobBooth did ( https://github.com/Esri/military-tools-geoprocessing-toolbox/issues/387#issuecomment-455248728 ), so for me this problem seems to be isolated to, and be a bug with, Pro 2.1. If you run "Apply Symbology From Layer" GP Tool with the same layer files they seem to show up correctly.

BobBooth commented 5 years ago

Using today's build on Pro 2.3, I am seeing the toolbox (PYT) LLOS tool and the LLOS tool from the Analysis toolbar both producing correctly symbolized and layer-described output. I think this is a Pro 2.1 issue.

BobBooth commented 5 years ago

Testing on Pro 2.1 and 2.2 with today's build I'm seeing correct symbology, but only the numeric value descriptions. In Pro 2.3 the layer descriptions are the correct "Visible" "Not Visible" verbose descriptions. As @csmoore mentioned, after running "Apply Symbology From Layer" GP Tool with the same layer files they show up correctly. I created four new LYRX files using Pro 2.2 and tested on Pro 2.2; the symbology was again correct, but the descriptions were still simply numeric. I suspect that this is a core issue with Pro 2.1 and Pro 2.2, which is fixed in Pro 2.3.

BobBooth commented 5 years ago

To Do: Log a known issue in the Release notes that on Pro 2.1 (and maybe 2.2?) the layer category descriptions are numeric (0, 1) rather than verbose (Visible, Not Visible). It works correctly on Pro 2.3

BobBooth commented 5 years ago

Added to Release notes for MT, republished from CMS, rebuilt Jenkins.