-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Intersection crosswalk options #993
Conversation
While working on this PR, I noticed a bug with the sidewalk width parameter where the sidewalk does not correctly align with the intersection if the intersection width/length is not the default value; I'll draft a fix in a separate PR. |
src/components/intersection.js
Outdated
|
||
const transformImageCrosswalk = (entity, length, rotZ) => { | ||
entity.setAttribute('rotation', { z: rotZ }); | ||
entity.setAttribute('scale', { y: length / 12, x: 1.5 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please define all three x y z. Although that somehow works on aframe 1.5.0, setting a partial vec3 is not a valid usage and sometimes won't work properly (setting missing values to 0 and not 1 as you expect for scale) on aframe 1.6.0 or later depending on if you insert the entity in DOM first.
src/components/intersection.js
Outdated
}, | ||
'crosswalk-raised': (entity, length, rotX) => { | ||
entity.setAttribute('rotation', { x: rotX, y: 90, z: 90 }); | ||
entity.setAttribute('scale', { x: length / 7, z: 1.5 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here please define full the vec3 with y: 1
src/components/intersection.js
Outdated
none: function () {}, | ||
'crosswalk-zebra': (entity, length, rotZ) => { | ||
entity.setAttribute('rotation', { z: rotZ }); | ||
entity.setAttribute('scale', { y: length / 12 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here please define full the vec3 with x: 1, z: 1
src/assets.js
Outdated
@@ -211,13 +211,17 @@ function buildAssetHTML(assetUrl, categories) { | |||
<a-mixin shadow id="temporary-traffic-cone" scale="1 1 1" rotation="0 0 0" gltf-part="src: #dividers; part: traffic-cone"></a-mixin> | |||
<a-mixin shadow id="temporary-jersey-barrier-plastic" scale="1 1 1" rotation="0 0 0" gltf-part="src: #dividers; part: jersey-barrier-plastic"></a-mixin> | |||
<a-mixin shadow id="temporary-jersey-barrier-concrete" scale="1 1 1" rotation="0 0 0" gltf-part="src: #dividers; part: jersey-barrier-concrete"></a-mixin> | |||
<a-mixin shadow id="street-element-crosswalk-raised" scale="1 1 1" rotation="0 0 0" gltf-model="url(${assetUrl}sets/uoregon/gltf-exports/draco/crosswalk-raised.glb)"></a-mixin> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably keep the original id because there may be users who have existing scenes with street-element-crosswalk-raised id
src/components/intersection.js
Outdated
cw1.setAttribute('mixin', 'markings crosswalk-zebra'); | ||
cw1.setAttribute( | ||
'mixin', | ||
'markings ' + CROSSWALKS_REV[crosswalklArray[0]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get rid of markings
now and just set the mixin to the crosswalk value
Towards having intersections with crosswalk options, this PR changes the crosswalk design of the intersection to use a selection instead of a boolean and adds the new crosswalks requested by Marc:
Implementation
intersection
component as a number that is mapped to the corresponding crosswalk type in the objectCROSSWALKS
defined inintersection.js
CROSSWALK_TRANSFORMS
object that defines a unique transformation that each crosswalk entity can have, so that they can all be oriented correctly.street-element-crosswalk-raised
to justcrosswalk-raised
for consistency with other crosswalks.