From e126dc3a5a3857b24246432153b0554d630b6234 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sat, 27 Aug 2016 15:26:15 -0700 Subject: [PATCH] [core] Fix updates of line and fill annotations --- src/mbgl/annotation/annotation_manager.cpp | 17 +++-- src/mbgl/annotation/annotation_manager.hpp | 2 +- src/mbgl/annotation/fill_annotation_impl.cpp | 19 ++--- src/mbgl/annotation/line_annotation_impl.cpp | 21 +++--- src/mbgl/style/style.cpp | 4 +- src/mbgl/style/style.hpp | 4 +- test/api/annotations.cpp | 68 ++++++++++++++++++ .../update_fill_geometry/expected.png | Bin 0 -> 1743 bytes .../update_fill_style/expected.png | Bin 0 -> 1481 bytes .../update_line_geometry/expected.png | Bin 0 -> 2091 bytes .../update_line_style/expected.png | Bin 0 -> 2794 bytes 11 files changed, 106 insertions(+), 29 deletions(-) create mode 100644 test/fixtures/annotations/update_fill_geometry/expected.png create mode 100644 test/fixtures/annotations/update_fill_style/expected.png create mode 100644 test/fixtures/annotations/update_line_geometry/expected.png create mode 100644 test/fixtures/annotations/update_line_style/expected.png diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp index 4e837d370..f1ddf9960 100644 --- a/src/mbgl/annotation/annotation_manager.cpp +++ b/src/mbgl/annotation/annotation_manager.cpp @@ -42,7 +42,7 @@ void AnnotationManager::removeAnnotation(const AnnotationID& id) { symbolTree.remove(symbolAnnotations.at(id)); symbolAnnotations.erase(id); } else if (shapeAnnotations.find(id) != shapeAnnotations.end()) { - obsoleteShapeAnnotationLayers.push_back(shapeAnnotations.at(id)->layerID); + obsoleteShapeAnnotationLayers.insert(shapeAnnotations.at(id)->layerID); shapeAnnotations.erase(id); } } @@ -54,18 +54,21 @@ void AnnotationManager::add(const AnnotationID& id, const SymbolAnnotation& anno } void AnnotationManager::add(const AnnotationID& id, const LineAnnotation& annotation, const uint8_t maxZoom) { - shapeAnnotations.emplace(id, - std::make_unique(id, annotation, maxZoom)); + ShapeAnnotationImpl& impl = *shapeAnnotations.emplace(id, + std::make_unique(id, annotation, maxZoom)).first->second; + obsoleteShapeAnnotationLayers.erase(impl.layerID); } void AnnotationManager::add(const AnnotationID& id, const FillAnnotation& annotation, const uint8_t maxZoom) { - shapeAnnotations.emplace(id, - std::make_unique(id, annotation, maxZoom)); + ShapeAnnotationImpl& impl = *shapeAnnotations.emplace(id, + std::make_unique(id, annotation, maxZoom)).first->second; + obsoleteShapeAnnotationLayers.erase(impl.layerID); } void AnnotationManager::add(const AnnotationID& id, const StyleSourcedAnnotation& annotation, const uint8_t maxZoom) { - shapeAnnotations.emplace(id, - std::make_unique(id, annotation, maxZoom)); + ShapeAnnotationImpl& impl = *shapeAnnotations.emplace(id, + std::make_unique(id, annotation, maxZoom)).first->second; + obsoleteShapeAnnotationLayers.erase(impl.layerID); } Update AnnotationManager::update(const AnnotationID& id, const SymbolAnnotation& annotation, const uint8_t maxZoom) { diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp index a0f25fdf5..a9afa0d52 100644 --- a/src/mbgl/annotation/annotation_manager.hpp +++ b/src/mbgl/annotation/annotation_manager.hpp @@ -73,7 +73,7 @@ private: SymbolAnnotationTree symbolTree; SymbolAnnotationMap symbolAnnotations; ShapeAnnotationMap shapeAnnotations; - std::vector obsoleteShapeAnnotationLayers; + std::set obsoleteShapeAnnotationLayers; std::set tiles; SpriteStore spriteStore; diff --git a/src/mbgl/annotation/fill_annotation_impl.cpp b/src/mbgl/annotation/fill_annotation_impl.cpp index fe520451f..0c69bc4fe 100644 --- a/src/mbgl/annotation/fill_annotation_impl.cpp +++ b/src/mbgl/annotation/fill_annotation_impl.cpp @@ -13,16 +13,19 @@ FillAnnotationImpl::FillAnnotationImpl(AnnotationID id_, FillAnnotation annotati } void FillAnnotationImpl::updateStyle(Style& style) const { - if (style.getLayer(layerID)) - return; + Layer* layer = style.getLayer(layerID); + FillLayer* fillLayer = layer ? layer->as() : nullptr; - std::unique_ptr layer = std::make_unique(layerID, AnnotationManager::SourceID); - layer->setSourceLayer(layerID); - layer->setFillOpacity(annotation.opacity); - layer->setFillColor(annotation.color); - layer->setFillOutlineColor(annotation.outlineColor); + if (!fillLayer) { + fillLayer = style.addLayer( + std::make_unique(layerID, AnnotationManager::SourceID), + AnnotationManager::PointLayerID)->as(); + fillLayer->setSourceLayer(layerID); + } - style.addLayer(std::move(layer), AnnotationManager::PointLayerID); + fillLayer->setFillOpacity(annotation.opacity); + fillLayer->setFillColor(annotation.color); + fillLayer->setFillOutlineColor(annotation.outlineColor); } const ShapeAnnotationGeometry& FillAnnotationImpl::geometry() const { diff --git a/src/mbgl/annotation/line_annotation_impl.cpp b/src/mbgl/annotation/line_annotation_impl.cpp index f18ca9fad..5ec5a2a49 100644 --- a/src/mbgl/annotation/line_annotation_impl.cpp +++ b/src/mbgl/annotation/line_annotation_impl.cpp @@ -13,17 +13,20 @@ LineAnnotationImpl::LineAnnotationImpl(AnnotationID id_, LineAnnotation annotati } void LineAnnotationImpl::updateStyle(Style& style) const { - if (style.getLayer(layerID)) - return; + Layer* layer = style.getLayer(layerID); + LineLayer* lineLayer = layer ? layer->as() : nullptr; - std::unique_ptr layer = std::make_unique(layerID, AnnotationManager::SourceID); - layer->setSourceLayer(layerID); - layer->setLineJoin(LineJoinType::Round); - layer->setLineOpacity(annotation.opacity); - layer->setLineWidth(annotation.width); - layer->setLineColor(annotation.color); + if (!lineLayer) { + lineLayer = style.addLayer( + std::make_unique(layerID, AnnotationManager::SourceID), + AnnotationManager::PointLayerID)->as(); + lineLayer->setSourceLayer(layerID); + } - style.addLayer(std::move(layer), AnnotationManager::PointLayerID); + lineLayer->setLineJoin(LineJoinType::Round); + lineLayer->setLineOpacity(annotation.opacity); + lineLayer->setLineWidth(annotation.width); + lineLayer->setLineColor(annotation.color); } const ShapeAnnotationGeometry& LineAnnotationImpl::geometry() const { diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index e04384a96..ae6bc14b8 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -154,7 +154,7 @@ Layer* Style::getLayer(const std::string& id) const { return it != layers.end() ? it->get() : nullptr; } -void Style::addLayer(std::unique_ptr layer, optional before) { +Layer* Style::addLayer(std::unique_ptr layer, optional before) { // TODO: verify source if (SymbolLayer* symbolLayer = layer->as()) { @@ -167,7 +167,7 @@ void Style::addLayer(std::unique_ptr layer, optional before) customLayer->impl->initialize(); } - layers.emplace(before ? findLayer(*before) : layers.end(), std::move(layer)); + return layers.emplace(before ? findLayer(*before) : layers.end(), std::move(layer))->get(); } void Style::removeLayer(const std::string& id) { diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp index 149a0f080..06a25385b 100644 --- a/src/mbgl/style/style.hpp +++ b/src/mbgl/style/style.hpp @@ -65,8 +65,8 @@ public: std::vector getLayers() const; Layer* getLayer(const std::string& id) const; - void addLayer(std::unique_ptr, - optional beforeLayerID = {}); + Layer* addLayer(std::unique_ptr, + optional beforeLayerID = {}); void removeLayer(const std::string& layerID); std::string getName() const; diff --git a/test/api/annotations.cpp b/test/api/annotations.cpp index b6f861f90..5b9fe7502 100644 --- a/test/api/annotations.cpp +++ b/test/api/annotations.cpp @@ -201,6 +201,74 @@ TEST(Annotations, UpdateSymbolAnnotationIcon) { test.checkRendering("update_icon"); } +TEST(Annotations, UpdateLineAnnotationGeometry) { + AnnotationTest test; + + LineAnnotation annotation { LineString {{ { 0, 0 }, { 45, 45 }, { 30, 0 } }} }; + annotation.color = { { 255, 0, 0, 1 } }; + annotation.width = { 5 }; + + test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); + AnnotationID line = test.map.addAnnotation(annotation); + + test::render(test.map); + + annotation.geometry = LineString {{ { 0, 0 }, { -45, -45 } }}; + test.map.updateAnnotation(line, annotation); + test.checkRendering("update_line_geometry"); +} + +TEST(Annotations, UpdateLineAnnotationStyle) { + AnnotationTest test; + + LineAnnotation annotation { LineString {{ { 0, 0 }, { 45, 45 }, { 30, 0 } }} }; + annotation.color = { { 255, 0, 0, 1 } }; + annotation.width = { 5 }; + + test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); + AnnotationID line = test.map.addAnnotation(annotation); + + test::render(test.map); + + annotation.color = { { 0, 255, 0, 1 } }; + annotation.width = { 2 }; + test.map.updateAnnotation(line, annotation); + test.checkRendering("update_line_style"); +} + +TEST(Annotations, UpdateFillAnnotationGeometry) { + AnnotationTest test; + + FillAnnotation annotation { Polygon {{ {{ { 0, 0 }, { 0, 45 }, { 45, 45 }, { 45, 0 } }} }} }; + annotation.color = { { 255, 0, 0, 1 } }; + + test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); + AnnotationID fill = test.map.addAnnotation(annotation); + + test::render(test.map); + + annotation.geometry = Polygon {{ {{ { 0, 0 }, { 0, 45 }, { 45, 0 } }} }}; + test.map.updateAnnotation(fill, annotation); + test.checkRendering("update_fill_geometry"); +} + +TEST(Annotations, UpdateFillAnnotationStyle) { + AnnotationTest test; + + Polygon polygon = {{ {{ { 0, 0 }, { 0, 45 }, { 45, 45 }, { 45, 0 } }} }}; + FillAnnotation annotation { polygon }; + annotation.color = { { 255, 0, 0, 1 } }; + + test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); + AnnotationID fill = test.map.addAnnotation(annotation); + + test::render(test.map); + + annotation.color = { { 0, 255, 0, 1 } }; + test.map.updateAnnotation(fill, annotation); + test.checkRendering("update_fill_style"); +} + TEST(Annotations, RemovePoint) { AnnotationTest test; diff --git a/test/fixtures/annotations/update_fill_geometry/expected.png b/test/fixtures/annotations/update_fill_geometry/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..8dbcc7795c075a99b058524434954f846df60c2a GIT binary patch literal 1743 zcmeAS@N?(olHy`uVBq!ia0y~yU<5K5893O0R7}x|G!U;i$lZxy-8q?;3=C|Yo-U3d z6^w7M-3@9s5NNpA_x}Gmt*VPgx5S+4$|gUq2-IyUnD(>n_E~;LCKe6>1&0QPk;$my zuDO?Ye}7$==#Pl6$L@ZQbzpfXwz^iR;i}rdCv~n5IL|!!FV=99z2@0Bmj^x9WR7zk zkSShMCax6FYjo6!X+n}ut+^&s?OyjdUk0~V?EAKHu*mrxUCZg<>fy70}^#T!1>2WxITI^KOChQEQ~jOls3a%HC28M|X#7Ispz;4{H}`>5C+-f1d+81gpU&xBIp?Zy#ey&2je#%N z^26K)jy=L%-}f>cneF*Xw$owVM^1-zl}m4|)Kf&SX%>=KbD*&&fuOiBU#kqoaQqgF0%;J9*7tih@;OcH3` z^^2?zb}ba;SRyLWux0V)kG=<@4{Uwzz_9M8rFBCC#}s}?hEs3nNP0N1oN7~KNXZF0*(LMH5vZz{_!59 zt$YR3hwDe`)x-oE{}!+&ynnc!6{y%Qg>gspk1Z1#IR5ZOGl*5axce&3f#sXrKcx{{ kIE>_iks5wK;L)i6v$cVD`HHtiz#59d)78&qol`;+0N4)EiU0rr literal 0 HcmV?d00001 diff --git a/test/fixtures/annotations/update_fill_style/expected.png b/test/fixtures/annotations/update_fill_style/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..84b854fd36ff3bf9f6448661ca5ef198cbff87fb GIT binary patch literal 1481 zcmeAS@N?(olHy`uVBq!ia0y~yU<5K5893O0R7}x|G!U;i$lZxy-8q?;3=FKzo-U3d z6^w7M9OPwC6ky)?^w)dIxbVXYiBcXmdKI;Vst0G(nkGXMYp literal 0 HcmV?d00001 diff --git a/test/fixtures/annotations/update_line_geometry/expected.png b/test/fixtures/annotations/update_line_geometry/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..d3b9399968a9e7690491c7915c5d8a34d48bbb2a GIT binary patch literal 2091 zcmeH|eM}o=9LKL+N6Q%NK_j9tID`OGuq4XrFw&QGD@# z*@u}uhE&gPn^(A>Hjd?|6|s^=UCw=5*R!AH_qU%GPC0%cCf;8QFTf(!ca4pWSN3RO zk)KvqY&JGpZx?hNi>Mg9H)_Qa*9xc4w7!^_nT2ub3>J(C&O)^3i{}vjTNCDuFn%U0 zYl`fWnPf!}dhInG&g)I8I&W6FAw!3dJI1gwHIcqu9)dQ}O9^y!d~j{`kEXy}&Iv!= zUHyQIiZDW|-gRLp{8hkEJc7!?OVrD7i1kFu8>k1}+1^-+A6B690JMVcOj=5D?i{74 zKf&fS%&{LlSO?uL!8%J`|7r?McI=MH2pke2!-~s`7(%3djCe-ZHVCe(3t+PEf#U9+ zt}OcGc6a=S2Iu#!@0z6T3>Z3!pyr4YvirfD*&$!k;Ay_kyyJa31#~Gm-p;WZMktYJ z=GbHjltR>9GCHw4N19&G{uIypU5x~pY{-w~~!YmvKP}f7`c>m#6hErWApvfL8BL-Np#R~18DwLRQvcl zea_zR+nQQuj-0w7v{#M*${T6A4Y7!wviz6jDTc#MQ$6|Y$BL?-7*q%t))vBW~1<2&_ z0_5awc@@-kVD$@_EOazO@%7>|LQHM4Y5{dk->mEH(VMySdSHG(s4KQ!QQz$)sciuP z<-p|Oc782oY5t_sMnL<1Udo?fo$`npn|jZr{w&SwzVii-D7-t?q)w;Y&a($rmGe-@ zY)0za=m@=3Re9;Elg%tW{oCh^2*4&(`2qWp`d0_4ob2S?CgUqBGkT}63PBpmAS?*M z&NfP^*<<}oo7%8e_--X{Q&z2S;wa8*ZLSfdqYM=xwg*-DD7?{)gmKW<>5UJ2&^1hX zbeZj-I}*lQF-XRC1u|G~a_|+m{JPN$r8E4YzfL0W2`Bzid7RPUH6B^;66oDGidQln&wD^JnEkhCk5h^HXfHD-6U?c%CKm>^zu1Xym z1_dVssda)7h&X@}1_ulgBvd6JV_04xU>FRsgc#s9U44JT`^}%uKKr}Q*?XV$Ip^%t zI|6*nO&6O20ATLB)hh@9Fh&yt;7yD}L{iD#e`d)+KAV9G`H~R;FumdHMcb8z8S67E z5z(!If%D$6j->*>X441XW)kd92ilwuJnOYP)^Cbv`OME@1hzR%*n8mR!2xNDzC zz9b$YIGC6j7Z)}aU^Hqnm zugE0i>@obda41G>Gudm(OR`NBO*o2PQ8ONIHf6XyoULlObrdP)_I&5GUsSA%kh%9* zVL*SVC-JkOMcCz7(`p|x@mU|X1&d#Mmnv12b&2;O0o4g)q=-A}`CBcQx8hDEjuar? z(r_(HlK3>V-uq*rf74;ljK`p{oE@!bwX+B3jhu9_6hZeP^RHwZeg$CMH?obwf?5M* zPPM|j3Umm+@an0%8l)gNt!kM^WCXniiO28lFmN@H04{^5lP z)+lU+kUT{@B@ywg3Ogh{OV-^lmnWYy27z)pd&r-ndZte#1Q%))1?bZRIT;3Ej2WZ0 zyxWLQ6`W>(!B#W%I>!A-$N`OOS=t20hSM({6@U?)9+Ez;% z2b)aiE62%1OK3d=3yy?q(R&p@_LY*m0!l}XJ28w&!hl~{$kNi-15xya#Xkmm6AX$P zhn4>m7P)8Wp0VDU5i3?9(_y@sM)o?0`a2UI_jKLTHh~7)PLMccLP*EwS>i8jj1f9h zMqy;D;hwX{zIT44sgl3=1!?6tEdJA~T+I_!|COHUhjjYgnR!%Z1*^{6=!bGOQ>?jh zDe4p)Nt0$;t8SKM@BRlpD_PoDsCL=?4=?-7j&I3Xn!;By6`Zy9)aU?^;JAq0?IK#O zP7?jpVb!$9$gXjp_Z>56sY8ZNv?)efYOO9IH(6!?Fu};BkgQG=?xr}O-$rPtNXsem zbEDxcF2Pi(ygZN}(fYZol(DFe5}m3DtG&2NgJEY^TS3U=843E@ZUqMPw3JOBAjj$a>PGsux4nJvuAg*cOqdD4 z-@}4bdfq|Zo8qdJv^B`27B9c@ZvuZ%rRt^v)O)AE&GbBn_MHE`J;uz4;bJUl{u1`w zvkr^6!JCBCc+JKB4ew!x?Me9ntT$u0aYg!^%f6VFV)N;-%Z=X}y>z_5BOh77#T{r~ zvpimTPOx)LlWSEX{4*B+G8jVU%2)G)vJQ=J%Zr`)V=~^54DJm(HGsu?v87A9J8vrT zFW%S8cZzxOwrnzja&HN^5L$0M9Hwx!n1##|LpSN$1=k%qeO5-r<1pYGYuWTx^81PI z>ERKRD7!GJ%Nixdp^$qDfFI3c)Jh689nK!HLx+5FoKm*0`J8H~(Z1CB8n2xV_JZ=H zm@U&EwqQ!HtqV!j_u2@PE&=dmBi=U_SF)v@t!~PoV}kAUe*IuXbAmnf_jwGrBO<-Z zfPN(KOM4gml}O8YT#F-3#hXv7SWoj*1@*knfmf~QL>pPmz~VQ+g2?Vph2n(jWuHCL z*jzWI`=*emv9t_r1N&Bt6ONJCjjUb>U);lmFb?^G|g zC*HY+C1sb~Qtv-ZX{fGaj&2BUa_5zK?g4GLK*&JlEgn~Fkf(7JNr7VS)_`bFJY$s7 zG2^4FR|%YiS68Hqy)jsuFbL6Vb~oWDpbvpqIMYh@wvyZxtC)HANL<8;P^3KFT+?F()9@*d;-&7dhR!&TZ#Y#@ zvz&Bv2iITc>1}e4mT55@4j~H_443;HU42VeJ|(7Fs2=+Fp|!%1vc*IL&sKMN&-(`_ zn>UJBuDr1)UJx}p85X#N9Ot^s6<6zDU}o|)&xI4Ym|fSgpqYKH=5FQCvVNvIw)cTT zrxbEjxO3Ikpu3S*o=C5JolJJPJ#jVNEq%x2>+>r&`ei(R4-$CQ(wdtE>e;-nFTFW4 z7y{LE0|+