[mapguide-commits] r10249 - in branches/4.0/MgDev: Common/Renderers Server/src/UnitTesting
svn_mapguide at osgeo.org
svn_mapguide at osgeo.org
Mon Nov 2 18:14:38 PST 2026
Author: jng
Date: 2026-11-02 18:14:38 -0800 (Mon, 02 Nov 2026)
New Revision: 10249
Modified:
branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp
branches/4.0/MgDev/Common/Renderers/KmlRenderer.h
branches/4.0/MgDev/Server/src/UnitTesting/CMakeLists.txt
branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp
Log:
Fix potential duplicate KML placemark names. Fixes #2762
Modified: branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp
===================================================================
--- branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp 2026-11-03 01:57:18 UTC (rev 10248)
+++ branches/4.0/MgDev/Common/Renderers/KmlRenderer.cpp 2026-11-03 02:14:38 UTC (rev 10249)
@@ -50,7 +50,8 @@
m_elevation(0.0),
m_extrude(false),
m_elevType(RS_ElevationType_RelativeToGround),
- m_metersPerUnit(metersPerUnit)
+ m_metersPerUnit(metersPerUnit),
+ m_placemarkName(L"")
{
m_kmlContent = m_mainContent;
m_pixelSize = METERS_PER_INCH / dpi;
@@ -102,7 +103,7 @@
if (m_featureCount > 0)
{
//end last feature
- m_kmlContent->WriteString("</Placemark>");
+ EndPlacemark();
}
m_kmlContent = m_mainContent;
@@ -125,6 +126,21 @@
}
+void KmlRenderer::EndPlacemark()
+{
+ //a placemark has one name at most, and an empty one shows as a blank entry
+ //next to the real name in Google Earth, so only write it when there is one
+ if (!m_placemarkName.empty())
+ {
+ m_kmlContent->WriteString("<name><![CDATA[", false);
+ m_kmlContent->WriteString(m_placemarkName, false);
+ m_kmlContent->WriteString("]]></name>");
+ m_placemarkName.clear();
+ }
+ m_kmlContent->WriteString("</Placemark>");
+}
+
+
void KmlRenderer::StartFeature(RS_FeatureReader* /*feature*/,
bool /*initialPass*/,
const RS_String* tooltip,
@@ -137,7 +153,7 @@
if (m_featureCount > 0)
{
//end last feature
- m_kmlContent->WriteString("</Placemark>");
+ EndPlacemark();
}
if (theme == NULL || theme->empty())
{
@@ -156,6 +172,7 @@
m_themeMap[*theme] = m_kmlContent;
}
}
+ m_placemarkName.clear();
m_kmlContent->WriteString("<Placemark>");
if ((tooltip != NULL && tooltip->length() > 0) ||
(url != NULL && url->length() > 0))
@@ -380,9 +397,12 @@
}
char buffer[256];
- m_kmlContent->WriteString("<name><![CDATA[", false);
- m_kmlContent->WriteString(mdef.name(), false);
- m_kmlContent->WriteString("]]></name>");
+ //the marker's name is its symbol name, not the feature's, so it is only used
+ //when nothing else names the placemark
+ if (m_placemarkName.empty())
+ {
+ m_placemarkName = mdef.name();
+ }
m_kmlContent->WriteString("<Point>");
WriteElevationSettings();
m_kmlContent->WriteString("<coordinates>");
@@ -402,9 +422,10 @@
double /*scaleLimit*/)
{
// char buffer[256];
- m_kmlContent->WriteString("<name><![CDATA[", false);
- m_kmlContent->WriteString(text, false);
- m_kmlContent->WriteString("]]></name>");
+ if (!text.empty())
+ {
+ m_placemarkName = text;
+ }
/* m_kmlContent->WriteString("<MultiGeometry>");
for (int i = 0; i < nlabels; i++)
{
Modified: branches/4.0/MgDev/Common/Renderers/KmlRenderer.h
===================================================================
--- branches/4.0/MgDev/Common/Renderers/KmlRenderer.h 2026-11-03 01:57:18 UTC (rev 10248)
+++ branches/4.0/MgDev/Common/Renderers/KmlRenderer.h 2026-11-03 02:14:38 UTC (rev 10249)
@@ -166,6 +166,7 @@
KmlRenderer& operator=(const KmlRenderer&);
void ProcessOneMarker(double x, double y, RS_MarkerDef& mdef, bool allowOverpost);
+ void EndPlacemark();
void WriteStyle(RS_FillStyle& fill);
void WriteStyle(RS_LineStroke& lsym);
void WriteStyle(double scale, const std::wstring& href);
@@ -198,6 +199,9 @@
bool m_extrude;
double m_metersPerUnit;
RS_ElevationType m_elevType;
+ // name to write for the placemark being rendered, collected from its label
+ // (or, failing that, from its marker) and written when the placemark closes
+ RS_String m_placemarkName;
};
#endif
Modified: branches/4.0/MgDev/Server/src/UnitTesting/CMakeLists.txt
===================================================================
--- branches/4.0/MgDev/Server/src/UnitTesting/CMakeLists.txt 2026-11-03 01:57:18 UTC (rev 10248)
+++ branches/4.0/MgDev/Server/src/UnitTesting/CMakeLists.txt 2026-11-03 02:14:38 UTC (rev 10249)
@@ -56,6 +56,7 @@
target_link_libraries(MgUnitTesting
${CPPUNIT_LIBRARIES}
+ MgRenderers${MG_VERSION_SUFFIX}
MgStylization${MG_VERSION_SUFFIX}
MgMdfModel${MG_VERSION_SUFFIX}
)
\ No newline at end of file
Modified: branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp
===================================================================
--- branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp 2026-11-03 01:57:18 UTC (rev 10248)
+++ branches/4.0/MgDev/Server/src/UnitTesting/TestKmlService.cpp 2026-11-03 02:14:38 UTC (rev 10249)
@@ -20,9 +20,13 @@
#include "ServerResourceService.h"
#include "ServerSiteService.h"
#include "FoundationDefs.h"
+#include "KmlRenderer.h"
#include "TestServiceFactory.h"
#include "catch.hpp"
+#include <string>
+#include <vector>
+
static STRING RemoveSessionIds(CREFSTRING content)
{
STRING newContent = content;
@@ -349,4 +353,134 @@
{
throw;
}
-}
\ No newline at end of file
+}
+
+//////////////////////////////////////////////////////////////////////////////
+// Kml renderer placemark naming (ticket #2762)
+//
+// These drive the renderer directly, in the order PointAdapter does for a point
+// feature that has both a point symbol and a label: ProcessMarker() and then
+// ProcessLabelGroup(). A placemark must carry at most one <name>, and never an
+// empty one, or Google Earth shows a blank name next to the real one.
+
+namespace
+{
+ // Splits a KML fragment into its <Placemark> elements
+ std::vector<std::string> GetPlacemarks(const std::string& kml)
+ {
+ static const std::string startTag("<Placemark>");
+ static const std::string endTag("</Placemark>");
+
+ std::vector<std::string> placemarks;
+ std::string::size_type start = kml.find(startTag);
+ while (start != std::string::npos)
+ {
+ std::string::size_type end = kml.find(endTag, start);
+ if (end == std::string::npos)
+ break;
+
+ end += endTag.length();
+ placemarks.push_back(kml.substr(start, end - start));
+ start = kml.find(startTag, end);
+ }
+ return placemarks;
+ }
+
+ int CountOccurrences(const std::string& text, const std::string& what)
+ {
+ int count = 0;
+ std::string::size_type pos = text.find(what);
+ while (pos != std::string::npos)
+ {
+ ++count;
+ pos = text.find(what, pos + what.length());
+ }
+ return count;
+ }
+
+ // Renders one point feature with a point symbol of the given marker name and,
+ // when label is not empty, with a label - this is what DefaultStylizer does
+ // for a point layer. The resulting KML is returned.
+ std::string RenderPointFeature(CREFSTRING markerName, CREFSTRING label)
+ {
+ KmlContent kmlContent;
+ RS_Bounds bounds(-87.8, 43.7, -87.6, 43.8);
+ KmlRenderer renderer(&kmlContent, bounds, 10000.0, 96.0, 1.0, 0);
+
+ RS_UIGraphic graphic(NULL, 0, L"");
+ RS_LayerUIInfo layerInfo(L"Parcels", L"Library://UnitTests/Layers/Parcels.LayerDefinition",
+ L"", true, true, false, L"", L"", true, true, 0.0, graphic);
+ RS_FeatureClassInfo classInfo(L"Parcels", L"Library://UnitTests/Data/Parcels.FeatureSource", L"Geometry");
+ renderer.StartLayer(&layerInfo, &classInfo);
+
+ LineBuffer lb(4);
+ lb.MoveTo(-87.7, 43.75);
+
+ renderer.StartFeature(NULL, true, NULL, NULL, NULL, 0.0, 0.0, RS_ElevationType_RelativeToGround);
+
+ RS_MarkerDef mdef;
+ mdef.name() = markerName;
+ renderer.ProcessMarker(&lb, mdef, true, NULL);
+
+ if (!label.empty())
+ renderer.ProcessLabelGroup(NULL, 0, label, RS_OverpostType_FirstFit, true, &lb, 0.0);
+
+ renderer.EndLayer();
+ return kmlContent.GetString();
+ }
+}
+
+TEST_CASE("KmlPlacemarkNameNotDuplicated", "[KmlService]")
+{
+ std::string kml = RenderPointFeature(L"", L"PARCEL 123");
+
+ std::vector<std::string> placemarks = GetPlacemarks(kml);
+ REQUIRE(placemarks.size() == 1);
+
+ std::string placemark = placemarks.front();
+ INFO(placemark);
+ REQUIRE(placemark.find("<name><![CDATA[PARCEL 123]]></name>") != std::string::npos);
+ REQUIRE(CountOccurrences(placemark, "<name>") == 1);
+}
+
+TEST_CASE("KmlPlacemarkOmitsEmptyName", "[KmlService]")
+{
+ // a point symbol whose marker has no name, and no label
+ std::string kml = RenderPointFeature(L"", L"");
+
+ std::vector<std::string> placemarks = GetPlacemarks(kml);
+ REQUIRE(placemarks.size() == 1);
+
+ std::string placemark = placemarks.front();
+ INFO(placemark);
+ REQUIRE(CountOccurrences(placemark, "<name>") == 0);
+}
+
+TEST_CASE("KmlPlacemarkLabelWinsOverMarkerName", "[KmlService]")
+{
+ // the marker name belongs to the symbol, not to the feature, so the label
+ // is the placemark name when both are present
+ std::string kml = RenderPointFeature(L"BlockSymbol", L"PARCEL 123");
+
+ std::vector<std::string> placemarks = GetPlacemarks(kml);
+ REQUIRE(placemarks.size() == 1);
+
+ std::string placemark = placemarks.front();
+ INFO(placemark);
+ REQUIRE(CountOccurrences(placemark, "<name>") == 1);
+ REQUIRE(placemark.find("<name><![CDATA[PARCEL 123]]></name>") != std::string::npos);
+}
+
+TEST_CASE("KmlPlacemarkMarkerNameWithNoLabel", "[KmlService]")
+{
+ // with no label, the marker name is all there is to name the placemark with
+ std::string kml = RenderPointFeature(L"BlockSymbol", L"");
+
+ std::vector<std::string> placemarks = GetPlacemarks(kml);
+ REQUIRE(placemarks.size() == 1);
+
+ std::string placemark = placemarks.front();
+ INFO(placemark);
+ REQUIRE(CountOccurrences(placemark, "<name>") == 1);
+ REQUIRE(placemark.find("<name><![CDATA[BlockSymbol]]></name>") != std::string::npos);
+}
More information about the mapguide-commits
mailing list