[mapguide-commits] r10257 - in branches/4.0/MgDev: Common/Renderers Common/Stylization Server/src/UnitTesting
svn_mapguide at osgeo.org
svn_mapguide at osgeo.org
Mon Nov 2 21:51:50 PST 2026
Author: jng
Date: 2026-11-02 21:51:50 -0800 (Mon, 02 Nov 2026)
New Revision: 10257
Modified:
branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp
branches/4.0/MgDev/Common/Renderers/KmlRenderer.h
branches/4.0/MgDev/Common/Stylization/Renderer.h
branches/4.0/MgDev/Common/Stylization/StylizationEngine.cpp
branches/4.0/MgDev/Common/Stylization/StylizationEngine.h
branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp
Log:
#2488: Signal the feature boundary with EndFeature instead of redefining initialPass
r10256 made a feature's style passes share the placemark opened by its initial
pass by redefining the initialPass that StartFeature receives as "the first style
pass that reaches the renderer", in each rendering pass. initialPass is not
private to the KML renderer: FeatureInfoRenderer and FeaturePropRenderer
(QueryFeatureGeometry and QueryFeatureProperties) ignore every pass that does not
set it, and FeaturePropRenderer appends one record for each pass it accepts, so a
layer needing a symbol or instance rendering pass would have reported every
feature once per pass in those results. It would also have changed, in an
unrelated service, whether a feature whose leading composite styles match no rule
appears in them at all.
The boundary is now signalled explicitly, and initialPass keeps the values every
other renderer already saw:
* Common/Stylization/Renderer.h: declare EndFeature(), the hook the interface
had reserved in a TODO, with an empty body so no renderer is obliged to
implement it.
* Common/Stylization/StylizationEngine.cpp/.h: StylizeVectorLayer() calls
EndFeature() once per feature, after its last style pass. Stylize() is void
again and initialPass is back to its original definition (first style of the
first rendering pass).
* Common/Renderers/KmlRenderer.cpp/.h: EndFeature() closes the placemark, and
StartFeature() opens one on the initial pass or when none is open - which is
what a feature re-stylized by a later rendering pass sees, so those passes no
longer merge features into one placemark. A pass that moves the feature to
another theme still starts one, because a placemark cannot span the folders
the themes are written into.
* Server/src/UnitTesting/TestKmlService.cpp: the composite-style cases drive
EndFeature() the way the engine does, a new
KmlCompositeStyleSeparatesFeaturesInLaterRenderingPass covers a pass in which
no style pass is initial, and the layer and feature class info the renderer
points at are held by a RailLayer fixture that outlives it.
Reverting the renderer to its pre-#2488 form fails three of the cases (3 vs 1 and
4 vs 2 placemarks, and 7485 vs 2495 for GetFeaturesKml over MultiCTS against the
single-style Rail layer). A composite layer whose first style matches no feature
was checked by temporarily filtering that style (reverted): one placemark per
feature either way.
Verified on ubuntu22 x64 in the Debug/ASan tree: KmlService passes with "All
tests passed (32 assertions in 13 test cases)" and no LeakSanitizer record, and
MappingService, RenderingService, Stylization and RasterStylization - the suites
that drive the stylization engine - all pass.
Modified: branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp
===================================================================
--- branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp 2026-11-03 05:32:43 UTC (rev 10256)
+++ branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp 2026-11-03 05:51:50 UTC (rev 10257)
@@ -122,6 +122,12 @@
}
+void KmlRenderer::EndFeature()
+{
+ EndPlacemark();
+}
+
+
void KmlRenderer::EndPlacemark()
{
if (!m_placemarkOpen)
@@ -166,11 +172,12 @@
}
//a feature is stylized once per composite type style, so this is called once
- //per style of the same feature and only the first of those passes is the
- //initial one. The placemark belongs to the feature, not to the style pass, so
- //the remaining passes add to the placemark the initial pass opened. A pass
- //that moves the feature to another theme cannot continue it, because a
- //placemark cannot span the folders the themes are written into.
+ //per style of the same feature and EndFeature() closes the feature when its
+ //last pass is done. The placemark belongs to the feature, not to the style
+ //pass, so only the initial pass - or a pass with no placemark open, which is
+ //what a later rendering pass sees - starts one. A pass that moves the feature
+ //to another theme starts one too, because a placemark cannot span the folders
+ //the themes are written into.
if (m_placemarkOpen && (initialPass || content != m_kmlContent))
{
EndPlacemark();
@@ -177,7 +184,7 @@
}
m_kmlContent = content;
- if (!m_placemarkOpen)
+ if (initialPass || !m_placemarkOpen)
{
m_placemarkName.clear();
m_kmlContent->WriteString("<Placemark>");
Modified: branches/4.0/MgDev/Common/Renderers/KmlRenderer.h
===================================================================
--- branches/4.0/MgDev/Common/Renderers/KmlRenderer.h 2026-11-03 05:32:43 UTC (rev 10256)
+++ branches/4.0/MgDev/Common/Renderers/KmlRenderer.h 2026-11-03 05:51:50 UTC (rev 10257)
@@ -61,6 +61,8 @@
RENDERERS_API virtual void EndLayer();
+ virtual void EndFeature();
+
RENDERERS_API virtual void StartFeature(RS_FeatureReader* feature,
bool initialPass,
const RS_String* tooltip = NULL,
Modified: branches/4.0/MgDev/Common/Stylization/Renderer.h
===================================================================
--- branches/4.0/MgDev/Common/Stylization/Renderer.h 2026-11-03 05:32:43 UTC (rev 10256)
+++ branches/4.0/MgDev/Common/Stylization/Renderer.h 2026-11-03 05:51:50 UTC (rev 10257)
@@ -76,11 +76,10 @@
///<summary>
/// Indicates the renderer should process all subsequent geometry
- /// as part of a single logical feature. When a feature is stylized by
- /// several composite type styles, this is called once per style and
- /// initialPass is true only for the first of those calls, which is
- /// where a renderer that groups a feature's output (the KML
- /// placemark, for example) opens its group.
+ /// as part of a single logical feature. A feature is stylized once per
+ /// composite type style, so this is called once per style pass and
+ /// initialPass is set only for the first style of the feature's first
+ /// rendering pass; EndFeature is called when the last pass is done.
///</summary>
virtual void StartFeature(RS_FeatureReader* feature,
bool initialPass,
@@ -92,11 +91,11 @@
RS_ElevationType zOffsetType = RS_ElevationType_RelativeToGround) = 0;
///<summary>
- /// Done processing current feature.
+ /// Done processing current feature - every style pass of it has been
+ /// sent. Renderers that group a feature's output (the KML placemark, for
+ /// example) close their group here; the rest have nothing to do.
///</summary>
- //TODO: is it needed -- next call to StartFeature can just imply
- //previous feature is done
- //virtual void EndFeature() = 0;
+ virtual void EndFeature() {}
///<summary>
/// Polygon features. Transformation to W2D space is done here. Also
Modified: branches/4.0/MgDev/Common/Stylization/StylizationEngine.cpp
===================================================================
--- branches/4.0/MgDev/Common/Stylization/StylizationEngine.cpp 2026-11-03 05:32:43 UTC (rev 10256)
+++ branches/4.0/MgDev/Common/Stylization/StylizationEngine.cpp 2026-11-03 05:51:50 UTC (rev 10257)
@@ -166,22 +166,17 @@
}
// stylize once for each composite type style
- bool featureStarted = false;
for (size_t i=0; i<numTypeStyles; ++i)
{
- // The initial pass is the one that opens the feature for the renderer,
- // which is the first style whose rules match - a style whose rules match
- // nothing never reaches the renderer at all, and the style passes that
- // follow the initial one belong to the same feature.
- bool initialPass = !featureStarted;
- if (Stylize(reader, exec, lb, compTypeStyles[i], &seTip, &seUrl, NULL,
- initialPass, instanceRenderingPass, symbolRenderingPass,
- nextInstanceRenderingPass, nextSymbolRenderingPass))
- {
- featureStarted = true;
- }
+ bool initialPass = (i == 0 && instanceRenderingPass == 0 && symbolRenderingPass == 0);
+ Stylize(reader, exec, lb, compTypeStyles[i], &seTip, &seUrl, NULL,
+ initialPass, instanceRenderingPass, symbolRenderingPass,
+ nextInstanceRenderingPass, nextSymbolRenderingPass);
}
+ // the feature's last style pass is done
+ se_renderer->EndFeature();
+
// free geometry when done stylizing
LineBufferPool::FreeLineBuffer(m_pool, spLB.release());
@@ -759,7 +754,7 @@
}
-bool StylizationEngine::Stylize(RS_FeatureReader* reader,
+void StylizationEngine::Stylize(RS_FeatureReader* reader,
FdoExpressionEngine* exec,
LineBuffer* geometry,
CompositeTypeStyle* style,
@@ -832,9 +827,8 @@
}
}
- // no rule matches, so the renderer never hears about this feature
if (rule == NULL)
- return false;
+ return;
// we found a valid rule - send a StartFeature notification
RS_String rs_tip, rs_url;
@@ -849,7 +843,7 @@
// up with no symbols - we're done in that case.
std::vector<SE_SymbolInstance*>* symbolInstances = &rule->symbolInstances;
if (symbolInstances->size() == 0)
- return true;
+ return;
double mm2sud = m_serenderer->GetScreenUnitsPerMillimeterDevice();
double mm2suw = m_serenderer->GetScreenUnitsPerMillimeterWorld();
@@ -1112,7 +1106,7 @@
// if the clipped buffer is NULL (completely clipped) just move on to
// the next feature
if (!lbc)
- return true;
+ return;
// otherwise continue processing with the clipped buffer
lb = lbc;
@@ -1259,8 +1253,6 @@
// free clipped line buffer if the geometry was clipped
if (spClipLB.get())
LineBufferPool::FreeLineBuffer(m_pool, spClipLB.release());
-
- return true;
}
Modified: branches/4.0/MgDev/Common/Stylization/StylizationEngine.h
===================================================================
--- branches/4.0/MgDev/Common/Stylization/StylizationEngine.h 2026-11-03 05:32:43 UTC (rev 10256)
+++ branches/4.0/MgDev/Common/Stylization/StylizationEngine.h 2026-11-03 05:51:50 UTC (rev 10257)
@@ -78,9 +78,7 @@
int saveHeight);
// Stylizes the current feature on the reader using the supplied composite type style.
- // Returns true if the renderer was given the feature - the style's rules can match
- // nothing, in which case no StartFeature() is issued for it.
- bool Stylize(RS_FeatureReader* reader,
+ void Stylize(RS_FeatureReader* reader,
FdoExpressionEngine* exec,
LineBuffer* geometry,
CompositeTypeStyle* style,
Modified: branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp
===================================================================
--- branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp 2026-11-03 05:32:43 UTC (rev 10256)
+++ branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp 2026-11-03 05:51:50 UTC (rev 10257)
@@ -519,10 +519,10 @@
return content;
}
- // Renders one point feature, calling StartFeature() once per composite type
- // style pass, in the order StylizationEngine does it. A feature whose leading
- // styles match no rule has no call for those styles, so initialPass marks the
- // first pass it actually reaches the renderer for.
+ // Renders one point feature: StartFeature() once per composite type style pass,
+ // in the order StylizationEngine does it, and then EndFeature(). initialPass is
+ // set only for the first style pass of the feature's first rendering pass, so a
+ // later rendering pass re-stylizes every feature with initialPass clear.
//
// ProcessMarker() rather than ProcessPolyline() because it is the one the
// renderer exports: the geometry adapters reach the other Process* methods
@@ -530,7 +530,7 @@
// when KmlRenderer lives in its own DLL (the Windows build).
void StylizePointFeature(KmlRenderer& renderer,
int stylePasses,
- bool initialPassOnFirst,
+ bool firstRenderingPass,
double y,
const RS_String* theme = NULL)
{
@@ -541,19 +541,28 @@
for (int pass = 0; pass < stylePasses; ++pass)
{
- renderer.StartFeature(NULL, initialPassOnFirst && pass == 0, NULL, NULL, theme, 0.0, 0.0, RS_ElevationType_RelativeToGround);
+ renderer.StartFeature(NULL, firstRenderingPass && pass == 0, NULL, NULL, theme, 0.0, 0.0, RS_ElevationType_RelativeToGround);
renderer.ProcessMarker(&lb, mdef, true, NULL);
}
+ renderer.EndFeature();
}
- void StartRailLayer(KmlRenderer& renderer)
+ // The renderer keeps a raw pointer to the layer and feature class info it is
+ // handed, so they have to outlive it for as long as it renders that layer.
+ struct RailLayer
{
- RS_UIGraphic graphic(NULL, 0, L"");
- RS_LayerUIInfo layerInfo(L"Rail", L"Library://UnitTests/Layers/MultiCTS.LayerDefinition",
- L"", true, true, false, L"", L"", true, true, 0.0, graphic);
- RS_FeatureClassInfo classInfo(L"Rail", L"Library://UnitTests/Data/Rail.FeatureSource", L"SHPGEOM");
- renderer.StartLayer(&layerInfo, &classInfo);
- }
+ RailLayer()
+ : graphic(NULL, 0, L"")
+ , layerInfo(L"Rail", L"Library://UnitTests/Layers/MultiCTS.LayerDefinition",
+ L"", true, true, false, L"", L"", true, true, 0.0, graphic)
+ , classInfo(L"Rail", L"Library://UnitTests/Data/Rail.FeatureSource", L"SHPGEOM")
+ {
+ }
+
+ RS_UIGraphic graphic;
+ RS_LayerUIInfo layerInfo;
+ RS_FeatureClassInfo classInfo;
+ };
}
TEST_CASE("KmlCompositeStyleWritesOnePlacemark", "[KmlService]")
@@ -560,8 +569,9 @@
{
KmlContent kmlContent;
RS_Bounds bounds(-87.8, 43.7, -87.6, 43.8);
+ RailLayer layer;
KmlRenderer renderer(&kmlContent, bounds, 10000.0, 96.0, 1.0, 0);
- StartRailLayer(renderer);
+ renderer.StartLayer(&layer.layerInfo, &layer.classInfo);
// one feature, stylized by three composite type styles
StylizePointFeature(renderer, 3, true, 43.75);
@@ -580,8 +590,9 @@
{
KmlContent kmlContent;
RS_Bounds bounds(-87.8, 43.7, -87.6, 43.8);
+ RailLayer layer;
KmlRenderer renderer(&kmlContent, bounds, 10000.0, 96.0, 1.0, 0);
- StartRailLayer(renderer);
+ renderer.StartLayer(&layer.layerInfo, &layer.classInfo);
// two features, two composite type styles each
StylizePointFeature(renderer, 2, true, 43.75);
@@ -605,8 +616,9 @@
{
KmlContent kmlContent;
RS_Bounds bounds(-87.8, 43.7, -87.6, 43.8);
+ RailLayer layer;
KmlRenderer renderer(&kmlContent, bounds, 10000.0, 96.0, 1.0, 0);
- StartRailLayer(renderer);
+ renderer.StartLayer(&layer.layerInfo, &layer.classInfo);
// a feature whose two composite type styles carry different legend labels:
// each theme is written into its own folder, and a placemark cannot be left
@@ -613,9 +625,17 @@
// open across folders, so the second theme starts a placemark of its own
RS_String themeA(L"Single Track");
RS_String themeB(L"Dual Track");
- StylizePointFeature(renderer, 1, true, 43.75, &themeA);
- StylizePointFeature(renderer, 1, false, 43.75, &themeB);
+ LineBuffer lb(2);
+ lb.MoveTo(-87.70, 43.75);
+ RS_MarkerDef mdef;
+
+ renderer.StartFeature(NULL, true, NULL, NULL, &themeA, 0.0, 0.0, RS_ElevationType_RelativeToGround);
+ renderer.ProcessMarker(&lb, mdef, true, NULL);
+ renderer.StartFeature(NULL, false, NULL, NULL, &themeB, 0.0, 0.0, RS_ElevationType_RelativeToGround);
+ renderer.ProcessMarker(&lb, mdef, true, NULL);
+ renderer.EndFeature();
+
renderer.EndLayer();
std::string kml = kmlContent.GetString();
@@ -629,6 +649,32 @@
REQUIRE(GetContentOutsidePlacemarks(kml).find("<coordinates>") == std::string::npos);
}
+TEST_CASE("KmlCompositeStyleSeparatesFeaturesInLaterRenderingPass", "[KmlService]")
+{
+ KmlContent kmlContent;
+ RS_Bounds bounds(-87.8, 43.7, -87.6, 43.8);
+ RailLayer layer;
+ KmlRenderer renderer(&kmlContent, bounds, 10000.0, 96.0, 1.0, 0);
+ renderer.StartLayer(&layer.layerInfo, &layer.classInfo);
+
+ // a symbol rendering pass re-stylizes every feature with initialPass clear, so
+ // nothing but EndFeature separates one feature from the next there
+ StylizePointFeature(renderer, 1, false, 43.75);
+ StylizePointFeature(renderer, 1, false, 43.78);
+
+ renderer.EndLayer();
+
+ std::string kml = kmlContent.GetString();
+ INFO(kml);
+ std::vector<std::string> placemarks = GetPlacemarks(kml);
+ REQUIRE(placemarks.size() == 2);
+ for (size_t i = 0; i < placemarks.size(); ++i)
+ {
+ REQUIRE(CountOccurrences(placemarks[i], "<Point>") == 1);
+ }
+ REQUIRE(GetContentOutsidePlacemarks(kml).find("<coordinates>") == std::string::npos);
+}
+
TEST_CASE("GetFeaturesKmlCompositeStylePerFeature", "[KmlService]")
{
try
More information about the mapguide-commits
mailing list