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

Window Size as an uniform. #28

Open
arhik opened this issue Mar 8, 2024 · 12 comments
Open

Window Size as an uniform. #28

arhik opened this issue Mar 8, 2024 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@arhik
Copy link
Member

arhik commented Mar 8, 2024

Currently all window sizes are (500, 500). Its hardcoded somewhere. For that to change we need to change to some shader code. Pass window size as an uniform as window size changes. Window uniform needs to be updated with window Events.

@arhik arhik added the enhancement New feature or request label Mar 11, 2024
@VarLad
Copy link
Member

VarLad commented Mar 11, 2024

We have already worked on an option to allow custom passing on window sizes (in WGPUCanvas.jl and WGPUgfx.jl)
We're not sure about what do you mean by shader code, but we're guessing that the object getting "stretched" is undesirable?

Can you better point us in this issue?

@arhik
Copy link
Member Author

arhik commented Mar 11, 2024

Yes. We need to avoid stretching.

@arhik
Copy link
Member Author

arhik commented Mar 11, 2024

Our determineSize call is stretching the drawn surface rather. We need to preserve aspect ratio while scaling. We need to pass window size data to the shader. Perspective Camera might need to be scaled appropriately. Window size data is not passed into the shader for this to happen.

Edit: we need to pass window size as uniform if we would like to update camera projection matrix in shader. Since our camera projection matrix is updated in CPU, setting aspectratio property in camera object would be enough like @ut112002 attempted below.

@ut112002
Copy link
Member

Could you point us to the file we should start looking into for the shader stuff.

Will we have to look into other repos as well or WGPUgfx is enough.

@arhik
Copy link
Member Author

arhik commented Mar 11, 2024

Here aspect ratio needs to be set based on window width and height. Also perspective matrix needs to be updated with change in height and width. I am not sure about perspective matrix update. But aspect ratio is good start. We may not have to update shader code. Looks like we can update perspective matrix through aspect ratio and width and height.

xS = 2*n/(r-l) # r-l is width

shaderSource = quote
struct $cameraUniform
eye::Vec3{Float32}
aspectRatio::Float32
lookAt::Vec3{Float32}
fov::Float32
viewMatrix::Mat4{Float32}
projMatrix::Mat4{Float32}
end
@var Uniform 0 $(binding) camera::$cameraUniform
end

@arhik
Copy link
Member Author

arhik commented Mar 11, 2024

updating aspect ratio field of camera object directly on event may work out of the box.

@arhik arhik assigned arhik and ut112002 and unassigned arhik Mar 11, 2024
@arhik arhik added this to the publish new release milestone Mar 11, 2024
@arhik
Copy link
Member Author

arhik commented Mar 11, 2024

r and l variables refer to right and left end of the window in unit sizes; same is the case with t and b referring to top and bottom

@ut112002
Copy link
Member

wgpu1.webm

I wrote the code to update the aspect ratio on every resize with

function attachWindowSizeCallback(scene, camera)
	callback = (_, w, h) -> begin
		@info w,h,scene.canvas.size
		camera.aspectRatio *= w/scene.canvas.size[1]
		camera.aspectRatio *= scene.canvas.size[2]/h
		scene.canvas.size = (w,h)
		WGPUCore.determineSize(scene.canvas.context)
	end
	GLFW.SetWindowSizeCallback(scene.canvas.windowRef[], callback)
end

and i ran the tri.jl example , is this desirable ?

@arhik
Copy link
Member Author

arhik commented Mar 16, 2024

looks like aspectratio is preserved. Are you using PollEvents ? Just asking since update is slow.

@arhik arhik moved this to In Progress in WGPUgfx feature set tracking Mar 16, 2024
@arhik
Copy link
Member Author

arhik commented Mar 17, 2024

I really appreciate you. There are many such trivial things that can be improved.

@arhik
Copy link
Member Author

arhik commented Mar 17, 2024

Base.setproperty!(camera::Camera, f::Symbol, v) = begin
setfield!(camera, f, v)
uniformData = camera.uniformData
if camera.uniformData != nothing
if f in propertynames(uniformData)
setproperty!(uniformData, f, v)
end
(viewMatrix, projMatrix) = computeTransform(camera)
uniformData.viewMatrix = viewMatrix
uniformData.projMatrix = projMatrix
updateUniformBuffer(camera)
end
end

setting aspectratio is everything to it. The above code clearly updates projection matrix too. This fix will be a good PR. Would you like to make PR with this.

I don't see other issues with this code. But, we might still need to pass window size as uniform since we don't know what users want. So you can raise a separate issue like preserve aspectratio and make a PR. We can keep this open untill shader code is adjusted. Shader code is also trivial. Just define struct like camera in getShaderCode function, and update buffer like camera.

@arhik
Copy link
Member Author

arhik commented Mar 17, 2024

@ut112002 summary: Raise separate issue and make a PR if you would like to. You take your time and submit PR.

Passing window uniform is slightly involved. Right now camera projection matrix is updated on CPU and Camera buffer is set. Another approach would be update camera projection matrix on GPU with change in window height and width. This would mean significant change in camera.jl and other shader code. In cases where we have multiple cameras, CPU version could have an overhead on buffer updates. This argument needs backing with experimentation.

@arhik arhik moved this from In Progress to Todo in WGPUgfx feature set tracking Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

3 participants