-
Notifications
You must be signed in to change notification settings - Fork 7
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
Support red skybox #40
Conversation
@@ -682,18 +682,27 @@ export class Scene { | |||
this.COMvisual.add(mesh); | |||
} | |||
|
|||
public addSky(): void { | |||
public addSky(cubemap: string | undefined): void { | |||
var cubeLoader = new THREE.CubeTextureLoader(); |
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.
If this asset comes from the websocket, we should pass the websocket loading manager here as well. Check for other cases where it was used.
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'm assuming that I'll add all skyboxes to fuel. I'd that okay?
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'm assuming that I'll add all skyboxes to fuel. I'd that okay?
It looks like this was already done, correct? That is, if I use this branch and add in a cubemap_uri
to my SDF, I get a red sky as desired. So I think that means everything was properly downloaded from fuel. Do I have that right?
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.
Yes, that sounds right
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
d96a6da
to
90c5b69
Compare
FYI, I rebased this onto the latest gzweb2, and added a few additional checks for null in 90c5b69. Without those, when not specifying a With that, I was able to test both without a |
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.
This looks good to me now as a temporary hack. I'm just waiting on @ColeOSRF to confirm it works for him before merging.
Nate if you're able to wrap back around to this later today could you throw update the red skybox with the one in this folder? It also contains the grey sky texture. |
Signed-off-by: Chris Lalancette <[email protected]>
See #42 |
Signed-off-by: Nate Koenig [email protected]
As the name suggest, this is a quick hack to get a red skybox working. This approach will be replaced by proper
.dds
loading when I have more time.Here is an example SDF file for testing: