iho-ohi / S-101-Documentation-and-FC

Repository issues of S-101 document and feature catalogue
23 stars 5 forks source link

System attribute flareBearing #121

Closed JeffWootton closed 2 months ago

JeffWootton commented 7 months ago

Should "system" attribute flare bearing be populated by the production systems every time (i.e. (1,1))? [Impacts clauses 19.2, 19.4 and 19.5]

Consider that this is dependent on whether there is a portrayal CSP that automatically orientates the light flare to a "default" value.

DavidGrant-NIWC commented 7 months ago

The portrayal CSP which evaluates flareBearing is LightFlareAndDescription.

The default orientation for the light flare is 135 degrees. This value is used if none of the conditions below apply.

TomRichardson6 commented 7 months ago

Propose that multiplicity of this attribute should be 0,1 it should only be present (populated by production tools) if the default value is not used. The Portrayal Catalogue logic can be used to ensure the default value is used when not populated. Propose no further action for edition 2.0.0. Close issue. A validation check could be considered for pointless encoding of default value, this may already exist.

alvarosanuy commented 7 months ago

Excuse my ignorance here but, does flareBearing really fit the definition of a 'System attribute'? In my mind, it would fit the criteria if it was only to declutter collocated all around lights, where predefined set of flare bearings exist and they are auto-populated by the production tool to ensure all of the flares are readable (similar to sector arc extension). Problem is, we also want to allow data producers to encode any bearing they want to assist them with other decluttering or cartographic scenarios. Consequently, the attribute can't be locked for editing.

I believe what we want is for flareBearing encoding to be assisted by production tools (i.e. allowing cartographers to rotate the flare with the mouse and until its position makes sense with the surrounding data and then get flareBearing auto-populated once the flare is dropped at a location). Having said this, sometimes you want the flare at exactly 090 and, by dragging and spinning the flare around, it may be difficult to 'stop' exactly at that value. Therefore, it should be possible for the user to overwrite the bearing the production tool came up with.

In the collocated lights example, all values could be preloaded by the production tool but they have to be editable if necessary.

TomRichardson6 commented 7 months ago

@alvarosanuy I agree that this should be editable, but production systems should populate with the default as a starting point. I'm not sure we have a clear definition of a system attribute and nothing makes these clear to production systems. Although all should be editable we may need to consider validation checks to ensure that inappropriate values are not used in some cases.

JeffWootton commented 7 months ago

Tend to agree with Alvaro. The 2nd part of the CSP as described by Dave actually performs the function originally intended to be performed by flareBearing.

image

DCEG current guidance:

image

DavidGrant-NIWC commented 7 months ago

Just note that the lights have to be in EXACTLY the same position, meaning they must reference the same spatial (point) object. Are there instances where you would want to adjust the flareBearing for lights which are close (at chart scale) but don't share the same spatial reference?

Also, if there are more than two flares than that would require manual adjustment.

alvarosanuy commented 7 months ago

I have the team checking what CARIS does at the moment but, my point is that it seems the implementation of S-52 LIGHTS CSP would be half done by ENC production software (collocated lights) and half by Portrayal (the default value). Being flareBearing a system attribute, shouldn't the production software do both? I guess, in that case, flareBearing multiplicity would have to be amended to 1,1 (currently is 0,1) .... It is important to highlight that the user would have the power to overwrite the value inserted by the production tool.

Consequently, portrayal would only be use to present the flare at whatever angle flareBearing is encoded.

I'm just a bit concerned that stakeholders may have different views on who would do what ...

TDYCARHugh commented 7 months ago

CARIS Production tools can populate initial values. The values can be adjusted by producers as needed.

tdepuyt commented 7 months ago

Esri allows flareBearing to be adjusted during production.

alvarosanuy commented 7 months ago

@tdepuyt & @TDYCARHugh

I guess my question is:

Are your tools processing S-52 CSP LIGHTS06 and populating flareBearing with 045 or 135 degree values (excluding directional lights, ranges >10M, etc). LIGHTS06 use ORIENT value to orientate directional lights' flares. Should this be done by production tools as well?

If flareBearing is going to be a 'system attribute', all the above should be done/calculated by the production tools and presented to the compiler. Compilers would then be able to change the flareBearing value for cartographic purposes if they will. A validation check should report when the orientation of the flare, for a directional light, does not match orientationValue at the time of creating the ENC file (i.e. in case the compiler changed the value).

Portrayal would then only be use to present the flare at whatever angle flareBearing is encoded. CSP LightFlareAndDescription wouldn't be needed. It would be processed by the production tools I guess.

Is this what we all aim for? If so it should be better documented in the DCEG and LightFlareAndDescription wouldn't be necessary in S101 PC (?)

The other option would be:

TDYCARHugh commented 7 months ago

In CARIS we have an implementation to populate flareBearing using the logic from LIGHTS06. The user can view and edit the flareBearing value.

JeffWootton commented 6 months ago

Based on the discussions at the S-101PT12 meeting, I have applied some additional guidance in the DCEG as follows:

image

image

Feedback on these draft changes would be appreciated.

JeffWootton commented 4 months ago

Based on all the above discussion, I see that there are 2 options:

1. Retain the CSP within the Portrayal Catalogue. This will result in the following system/modelling requirements:

2. Remove the CSP from the Portrayal Catalogue and mandate flareBearing for all lights. This will result in the following system/modelling requirements:

Personally, my preference would be for Option 2 as this option removes the requirement for a CSP in the Portrayal Catalogue. However, I would not argue against Option 1 if it is considered to be the better option for implementation.

TDYCARHugh commented 4 months ago

My vote is for option 1. The original intention was to avoid having to implement the CSP but the code is already in the S-101 portrayal (recently commented out, easy to reactivate). If the attribute is populated then the portrayal will have less to do but the default will be the same as in S-52. There may be cases where the producer would want to set it to something different and having the optional attribute allows them to do that.

TomRichardson6 commented 3 months ago

Also support option 1 this avoids the dataset carrying values that are not needed and for the reasons @TDYCARHugh states.

DavidGrant-NIWC commented 3 months ago

Note that option 1 reverses a previous decision. The CSP was removed in https://github.com/iho-ohi/S-101_Portrayal-Catalogue/issues/361

image

alvarosanuy commented 3 months ago

Final decisions wouldn't have much impact on data producers but, at this point in time, I would prefer Option2 as is the one that is currently implemented in DCEG & PC.

tdepuyt commented 3 months ago

I would prefer option 1 because it would allow for compilers to change the rotation for cartographic reasons. However option 2 is also acceptable if required.

TDYCARHugh commented 3 months ago

If I am not mistaken the decision at S-101 PT13 was to go with option 1. @JeffWootton please confirm.

JeffWootton commented 2 months ago

@TDYCARHugh this is correct (Decision related to Paper S-101PT13-06.1 and Action S-101PT13/26 refers).

Close Issue pending completion of S-101PT13 Action by NIWC.