Skip to content
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

discussion ticket - hatching texture regression #457

Closed
kfarr opened this issue Jan 17, 2024 · 4 comments
Closed

discussion ticket - hatching texture regression #457

kfarr opened this issue Jan 17, 2024 · 4 comments

Comments

@kfarr
Copy link
Collaborator

kfarr commented Jan 17, 2024

Context:

  • Previously the hatch mark image texture used for the divider segment type was incorrect. See hash striping too dense #24
  • I tried to fix hatch mark texture by replacing the underlying texture image Fix hash marks #455
  • It looks nice for new scenes importing a street from streetmix
  • But for old scenes it didn't work -- it just showed a white area where dividers should be, Further it would generate an error when attempting to save these, something about a null value. (This also might be related to the textures that are missing before this hotfix) so then I did a hotfix see Fix hash marks hotfix #456.
  • But, now for old scenes it still does not look good (see picture)
    image
    instead we want this:
    image

issue:

  • in our scenes we are saving material component information
  • the material component stores the texture path directly, making it hard to update

question:

  • should we consider letting users modify scenes only through a higher level street component that stores data in the same form as streetmix json with segment type and variants on those segments? that would empower us to make updates to textures or other elements of a segment without needing to modify user scene data to support the updates.
  • should we start using the versioning system? Even if we can't fix it for users we can at least let them know that their file is an old version that may not be supported and instructions on how to fix.

workaround:

  • you can update directly update the json of a 3dstreet to adjust the rendering of the stripes in an old scene to match the new style
  • I use firefoo and have access to the production database
  • For the scene in question I edit the striped buffer entity data directly

Here is an example of before and after:

Before:
image

After:
image

@Algorush
Copy link
Collaborator

Algorush commented Jan 17, 2024

the material component stores the texture path directly, making it hard to update

In theory, this should not happen. Only if user change the parameters of the materials on the scene manually. or the component data was changed in the Js code during scene initialization. Because I made a check in json-utils that if the component information is present in the mixin that is applied to the element, then do not save it in JSON. But it seems that this does not always work correctly, I can see this example with markup

@Algorush
Copy link
Collaborator

Algorush commented Jan 17, 2024

should we consider letting users modify scenes only through a higher level street component that stores data in the same form as streetmix json with segment type and variants on those segments? that would empower us to make updates to textures or other elements of a segment without needing to modify user scene data to support the updates.

But what if the user wants to change the segment elements - any of the components - position, rotaion, material, etc? Then this information still needs to be saved in JSON.
But we can make a high-level component for streets/segments and use information from it to initially load the scene. And store information in JSON only for elements that have been changed or added by the user. I'm still thinking about how this can be done, but it would greatly reduce the JSON size. Maybe we don’t even have to create additional components, but use streetmix data.
And there is also a question - what to do with animated elements that move around the scene. We can offer random animation if it is enabled. That is, do not save the position of all animated objects, but generating them randomly while scene load

@Algorush
Copy link
Collaborator

Algorush commented Jan 18, 2024

the material component stores the texture path directly, making it hard to update

In theory, this should not happen. Only if user change the parameters of the materials on the scene manually. or the component data was changed in the Js code during scene initialization. Because I made a check in json-utils that if the component information is present in the mixin that is applied to the element, then do not save it in JSON. But it seems that this does not always work correctly, I can see this example with markup

No, in this case with dividers saving data works correctly. Alternatively, to fix it on older scenes, we could add repeat information in material to the divider mixin rather than aframe-streetmix-parser.js.
...No, I tried adding data to the mixin inside assets.js, but the data is applied to the element from JSON. In the case of divider, the material:repeat property is calculated based on the street length.
I can add a check to json-utils when loading from JSON - if the property is present in the mixin, then use the value from the mixin. Then using repeat in the mixin inside assets for the default street length can make a better display option for the divider

@Algorush
Copy link
Collaborator

But we can make a high-level component for streets/segments and use information from it to initially load the scene. And store information in JSON only for elements that have been changed or added by the user. I'm still thinking about how this can be done, but it would greatly reduce the JSON size. Maybe we don’t even have to create additional components, but use streetmix data.

I think it's a good idea to do this. This will eliminate the need for versioning in many cases

@kfarr kfarr closed this as completed Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants