From 17306669c2d9769120b86e73876a58a76f12a624 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Fri, 6 Jul 2018 12:30:31 -0700 Subject: [PATCH] Fix issue #6919: bring gl-js collision handling closer to gl-native - Don't place features that _have_ text but don't have collision boxes - Create a single collision box even when the length of a label is less than half the box size Test updates: - Add debug collision circles to line-center to verify collision behavior - Add regression test that exercises case where a line label is almost exactly the same length as its line --- src/symbol/collision_feature.js | 2 +- src/symbol/placement.js | 11 ++-- .../mapbox-gl-js#6919/expected.png | Bin 0 -> 8227 bytes .../regressions/mapbox-gl-js#6919/style.json | 62 ++++++++++++++++++ 4 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 test/integration/render-tests/regressions/mapbox-gl-js#6919/expected.png create mode 100644 test/integration/render-tests/regressions/mapbox-gl-js#6919/style.json diff --git a/src/symbol/collision_feature.js b/src/symbol/collision_feature.js index a4c76c27435..660fe68b2e0 100644 --- a/src/symbol/collision_feature.js +++ b/src/symbol/collision_feature.js @@ -105,7 +105,7 @@ class CollisionFeature { bucketIndex, overscaling) { const step = boxSize / 2; - const nBoxes = Math.floor(labelLength / step); + const nBoxes = Math.floor(labelLength / step) || 1; // We calculate line collision circles out to 300% of what would normally be our // max size, to allow collision detection to work on labels that expand as // they move into the distance diff --git a/src/symbol/placement.js b/src/symbol/placement.js index 56be8fcea15..ae3dc4fba9f 100644 --- a/src/symbol/placement.js +++ b/src/symbol/placement.js @@ -153,16 +153,16 @@ class Placement { const partiallyEvaluatedTextSize = symbolSize.evaluateSizeForZoom(bucket.textSizeData, this.transform.zoom, symbolLayoutProperties.properties['text-size']); - const iconWithoutText = !bucket.hasTextData() || layout.get('text-optional'); - const textWithoutIcon = !bucket.hasIconData() || layout.get('icon-optional'); + const textOptional = layout.get('text-optional'); + const iconOptional = layout.get('icon-optional'); const collisionGroup = this.collisionGroups.get(bucket.sourceID); for (const symbolInstance of bucket.symbolInstances) { if (!seenCrossTileIDs[symbolInstance.crossTileID]) { - let placeText = symbolInstance.feature.text !== undefined; - let placeIcon = symbolInstance.feature.icon !== undefined; + let placeText = false; + let placeIcon = false; let offscreen = true; let placedGlyphBoxes = null; @@ -223,6 +223,9 @@ class Placement { offscreen = offscreen && placedIconBoxes.offscreen; } + const iconWithoutText = textOptional || (symbolInstance.numGlyphVertices === 0 && symbolInstance.numVerticalGlyphVertices === 0); + const textWithoutIcon = iconOptional || symbolInstance.numIconVertices === 0; + // Combine the scales for icons and text. if (!iconWithoutText && !textWithoutIcon) { placeIcon = placeText = placeIcon && placeText; diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#6919/expected.png b/test/integration/render-tests/regressions/mapbox-gl-js#6919/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..24f13a6950d4237aaa2c622bfa1bb7d05a2a6cd5 GIT binary patch literal 8227 zcmeHMc|6o>+t?jE zDyS9&w@h|_n%wcxiTSDTn|Iu1mRf5>g7_Y};K#9^YIjcju47nbv3~>krTK89`k_c$ zb;$WiKx|+RNSR;bm)< zt&H=)jtFDV1C#x=N$KgK#R1IxNZZrwdo`42JG{jB`T4mNJn!iqEO~XQ4FQ#~pYg6h zvI^#H+POUUzDVn%SH3Vjd-hCz=94+XQ=8Qf7xsCmCDUVRysRbLo|jo+YDcKz!hXY7 zcJV1GjA--Qw|C&FiFtX^BO{kxM>{m^?ZqO_x$Z_31qIxQB5GhQc3G(@?Oav-v4ET@ z^2tagf})R)50|1h2km`1yx0AE`2r9AO0K5+ZM3Bq zl(Ea{R<#eSyF-;~3Vjyc2jTV_L zc5}=0n!P-d)4lyKMs|L@9w#gM?@^T9xfaW<|Mw^|8(dB37A4}I+ct|n^yu5Yo$KhY zwzfhhOJCWE$P1WH+)7~n@b`8$9sgQF(_@?!Ls{3{-mc;1CY|RtYE|sdXNva1!UD28jhFcW74jB0SXJNISsTJF zmow99OZoQgG5BkhSP>Vpsw|r_HkeT`&Rcx8-HkVHsYkFS*GaJ1grLC4E}8&@LTNc` z5#fU6O&q8GyF)z>y+%wMo@xwz&JEjRpHCD@&nJQPWaZ>&ENA&u$T_U**F;NQGW$*y zeff2u$7El9b933!{3M+U160k;&1xnlym8Az))oDepkEzOq%T#b)6z-NtsQul(sOR@ zC1~_3&dM~Uo-K}%aT*vM?f)YsO&Me&`91`qn$qAsEjbSSgef(``)Ao5l_l&Z>I8yP zvyl@MBARNriHmyf>E%Tk9`5Z5;C*^(e_d_UsW@nvhWcup>f6~5_C_*G;fF`x=a$YbRV%}Yh-rscM%)nWJ78wv| zuo|L#B#eFgYnN2R><|dRJJr$BnLdlI3yX{MjV8OG--GgiKq{ zim@RZ6nILe=%@EpnXHH#W?#0X3*^)tER8|?7{ETBDSFS$8vpo*cdw9%DTJh5XJITT zua|n}8VHndVKXs->N;61?F3`;J&V?J^Rn%)>^n69Y3X)n7KF6Gs~z|SYN}B#DO^Z@ zqA}e!JXX$)=DyQ|%{u#8H(#gK3>K|h*yU|i5n|T!6TL%i*`e32tr;nz`ZlGTXvLrU zvNTz%2D_2p@5JD#L68hz=jC~<%=ho+;CS@p$qjh0)o*POyV!9=G{=s?7d>JW%fEZ6 z?Nx@$px(xf8|OYIDVBUp);U|`tKg@+27%D6e+yfnX3Fv6yBjnYt+)VX4s~UbPIXQ9 zpdV6me4h%*cT2Oemk1i2iygBZ931CIKIa;_wY#ux-SDDC7-?D>umdl<(ZJBKrO-zX z0eb%YRRBv0{H}!nf(X#9=4MvWf?lWu)w9h>ztw20Kf+0RSwg0(QH7D$a zUH+D!_R9v z{jKq{W7TREQELC#xuAt3uRr3{r;hA_lri=mS4HZNE_aMN%#E2sLsHFl(Bef?`@&9FEGyc*Tl-t_k?ZRzTL4u){Ru#kIIIN3ldX9myeV{HT}Wf*fS8J z1Q`QD3aEM}J*=vuQ20s^Z{kS4Pa}_5lRG=LATt#g7o!cUC7Ta<&DeoTD9N?Hv_VO~ z80St1yIQx6IOAz!UDBqTJq!T2{YD& zQaYm6kcfrPzE}^G6qPMREVq-8ewsyZWqAJF*C6ltDwDozn|RJc2@y7YbqA_JhFyy| zNRYs+4Laq>EDmK-1_z%&fj<_oJ;QC(*q1s>h?BH!x?9zI=9S%U1qFo|?}-Y8LYWS% zSm#i$%AR4*h_+Damc0_&s z>6wxCDoAn_uU!UnAW|TxdmLqIWCw+jNalPhBxhwwNlAycOWA0QFAS!GFYusM?QWgb z&^Ayg6uyWmhYpW^giV17VB=uEFhxl5w;=gV;pO`aVR`$sZiC-VWqss(=bff=~F| z>wAeOBPPa*nJ0toy>AqF&Au=z*@{xQr>7_A;Oy#(isP~bNVCPd>z$vlQO;cyrn3a< zEVOt5s0}c9c3ew~IoBw}HZ6A}vX&so!qS@WG5J8+o(C#JZ(HYz6p}nFZV4+^IEMl3 z0iFB8XRiY~0@*w8+7=&T#AguDNb*yUqDaf*8t+wgo5rp{&vxhKJ`y0%PCyVw3Mpvix|!Y5au?qi5RFzMemEQ7KhcKjzl5E@RTAH+^ZWH*~a9GCgt)cDusZ} zVGs>!S$e??Cn!%-s5gJI<`{@3s0X4ICG0+8_56E^mOFm@xcdSn$>Upc5^6GRE?&e$ zSCgu(Dy%ZS2afTg{+2g&2UOM!I71F+ch;aG2givwb#*(Km6i^no@%nMnxD2Rci~$K zA89W{6P%FtXvw9e5m+17j|@bB?JagFy8~j9f=PSG=&S zI`jPe8bGh6MiY({GQ%$$plK#kq0A>Zq-JJ@!E;nqLVc(B{r&xE#)4*}rHG)!Zggc$ zY-}vVBqZ#d9un%d$y8Q=W4%M=fu!fqTHhBvSEEFP67aZ7^HqY%ctH5lNvPOK;T~1RUELJhsTP#vb;3TOFf+3;VEPtRbG*rCPQ(SB*dq%=Y(!pLz8U07>t!AqCWks8%aY@qHz)q~+* zn|+6y-OK8qs3BS?i%Zp=OH8P9wX?ee9AOQ8F@^}W*#@&2+>Kg%?|=?UW8jC~g2tYd zef|LfXgM?r7Vhp3!2h5O05%|OU41#JUC>Q3Z8lF0lWhR|pscR(kXh z)!dxee@z4Z}mwX}*L=m@2G}JQ5(|)CYZ{dl_^qm7pL9imp(+zp}(V(n}#~ zCfV59^2ch1X(r*58WOY*z+%Pypd{si%<@zNI{l`k1jYTXdyq9q6?*6rGosJp%xPh^ zWWAJx1{-LcDb7)^UP-}(-Pj%0;me?!;bqfy1+LdW=kg00qcfF4>8FI*4zLBmCM!K! zD>uzD%Hf3ZtD)f@Sne#V@nZr-pS%d28Ai zqm1Xt8R3cLQ9>MjDcFWC3ry5wWq*NaixY&CYOoMEz!PW%A+dH0t0^t)1o6!>&>pqa zrCN}OOWyr1H0P@@G&D6GmyPYar=$5BsUrnSQL^r1-Gks8%E`KB&1vUwQJ)~0lW$&u z2tnDguO_xCuV8tmZ516<^e(kYfvDwbiW55l7rJzA z;qJZR`WI|$_`*-rlgY{_PcnMVj##$5u*5~>(0EEgzG-c1v$41Dr_L6P`h)_A5G5!f z&J9^Boy<660ZAEvptE|Jl?7@cBNADe?DNMrCTa(2C%-Q*-%OjR(D)N^*#Bawug{*W zd;9)RpCsDznuzAFdWKqyZraKXO9x7i*z%TcdiU~PL%V9f?2l|-?b3Oi$`K3 z)k*3Cc!4l*!KmnHn8Y|%9>8((R`!bj3-7Pf%_d2SiGC1Dpef2laKi-dXJBA(3vyqS z5mGt937nfYmZzI1FE?Hccg$7KHF|0!sv{bK^xhv)b+5`2_8)hT`LhfX($ZlxVIclD z;EVF1!5}2jVCLApJDxV#AAaJ_ojX_HYLRpzk&Z%!Ma3>iEg~W!F>nomBj=%_x18-R zuPM*dPdX{uW$gh~;sA}S?_4pjHi$9PVoiTw?B2D3RDrFqAocZG{bd3jEQUf|c+2by zb~m%IxVdTxxU@4SEW6OT0sx|~2QbjeAp3(c;N%DO76rrhqH0ZL1zzW1d&y~BPMX(qdj@!w}fhw5a zbaYtYQrpO*Hypcb%c@2W(8lrOyYUc# zO?pkJ(tOp-D(h~U-c=KK&jtCP~w z<4-kXZ>PC2O`Vh$k=DY_~DC@n4hdEz~r*SV@3Dx?>)DnEVV;9aN-xb?#Zbm-{{ zeb+;t=lkT?v2~^#yZrgSe~4RFvOXRdsDJYDqU6OaPPGuOc)gUBb(tzx+jaKm%N;R*5QaZf?E`4RHK6Ej#TQ z>sg>#V4P%}egw+EyF=J*$Nuu6DNDir&yV0jIw4FG^YiMzS@`Zy!VjE)e+ZY1G7Tp{ z?xb*8Kw6rzPvYaR{Tr>h zw32nVg4`^q`DYP8h(lA}E%KlIrym^0|I`1s{C^exga3YBRQgRMBn7u^e>42+&wy9e KR7q1lfBm1keSIka literal 0 HcmV?d00001 diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#6919/style.json b/test/integration/render-tests/regressions/mapbox-gl-js#6919/style.json new file mode 100644 index 00000000000..a0a7b88d57e --- /dev/null +++ b/test/integration/render-tests/regressions/mapbox-gl-js#6919/style.json @@ -0,0 +1,62 @@ +{ + "version": 8, + "metadata": { + "test": { + "debug": true, + "collisionDebug": true, + "description": "'Carribean Sea' crosses a tile boundary, but we don't draw the tile boundary in the test because JS and Native render tile boundaries differently.", + "height": 256, + "width": 1024 + } + }, + "center": [ + -73, + 15 + ], + "zoom": 4.5, + "sources": { + "mapbox": { + "type": "vector", + "maxzoom": 14, + "tiles": [ + "local://tiles/mapbox.mapbox-streets-v7/{z}-{x}-{y}.mvt" + ] + } + }, + "glyphs": "local://glyphs/{fontstack}/{range}.pbf", + "layers": [ + { + "id": "background", + "type": "background", + "paint": { + "background-color": "white" + } + }, + { + "id": "line-center", + "type": "symbol", + "source": "mapbox", + "source-layer": "marine_label", + "layout": { + "text-field": "{name_en}", + "symbol-placement": "line-center", + "text-allow-overlap": true, + "text-size": 50, + "text-letter-spacing": 0.5, + "text-font": [ + "Open Sans Semibold", + "Arial Unicode MS Bold" + ] + } + }, + { + "id": "line", + "type": "line", + "source": "mapbox", + "source-layer": "marine_label", + "paint": { + "line-width": 1 + } + } + ] +}