Skip to content

Conversation

@LANDAISB
Copy link
Contributor

@LANDAISB LANDAISB commented Nov 6, 2025

No description provided.

@LabordePierre LabordePierre self-assigned this Nov 7, 2025
Copy link
Contributor

@LabordePierre LabordePierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing bugs and may be new usages to cover.


layerB := geoViewManager createDomainObjectsLayer: #layerB.
geoViewManager addLayer: layerB.
self assert: layerB notNil.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

place assertion before adding


layerC := geoViewManager createDomainObjectsLayer: #layerC.
geoViewManager addLayer: layerC afterIndex: 1.
self assert: layerC notNil.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

place assertion before adding

GeoViewManagerImpl >> addLayer: aLayer afterIndex: anIndex [

aLayer name ifNil: [ ^ nil ].
self configureLayer: aLayer.
Copy link
Contributor

@LabordePierre LabordePierre Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated code with addLayer: method. addLayer: should call addLayer: afterIndex: (see AbstractGeoViewElement>>addLayer:)

"when the shape haven't coordinates, that mean the shape is managed by a parent (i.e. a dComposite), return an offset if exists"
aDShape coordinates ifNil: [ ^ offset ].
self processor ifNil: [ ^ nil ].
self processor projection ifNil: [ ^ nil ].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

projection cannot be nil here, this is a bug to resolve. I cannot accept because this is dangerous.


self layers do: [ :e |
e haveDomainObjects ifTrue: [ e updateObjects: anUserObjectList ] ]
self flag: 'TODO: Add a mutex for accessing layers collection'.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot accept, this is a bug to resolve.


| centerGeoPoint level tilesXY tilesXYBuffer findAtLeastOneTile realLevel|

self surfaceMap ifNil:[^self].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same problem : this is a bug to resolve. but I know this class is dirty, but maybe previous problems was related to the same reason.


self layers do: [ :e |
e haveDomainObjects ifTrue: [ e updateObjects: anUserObjectList ] ]
self flag: 'TODO: Add a mutex for accessing layers collection'.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot accept, this is a bug to resolve.


| centerGeoPoint level tilesXY tilesXYBuffer findAtLeastOneTile realLevel|

self surfaceMap ifNil:[^self].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same problem : this is a bug to resolve. but I know this class is dirty, but maybe previous problems was related to the same reason.

@LabordePierre LabordePierre added the pending Waiting for something label Nov 13, 2025
@LabordePierre
Copy link
Contributor

@LANDAISB is this Pull Request again valid?

@LANDAISB
Copy link
Contributor Author

Don't know yet, I want to check if mutex on layers fix all these problems. I have to test it on projects

@LabordePierre
Copy link
Contributor

Ok, stay "pending"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending Waiting for something

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants