szeged / webrender

A GPU-based renderer for the web
https://doc.servo.org/webrender/
Mozilla Public License 2.0
45 stars 7 forks source link

Update gfx-hal to 0.3 #306

Closed imiklos closed 4 years ago

imiklos commented 4 years ago

We found a shader compilation error on Metal.

The error and backtrace: https://gist.github.com/imiklos/d35e199fdd8ab79b76d10272e183569b

The generated shader code: https://gist.github.com/imiklos/5451905cefddc6c6a28f2017eed0891a

zakorgy commented 4 years ago

From the error it looks like there is an overflow when calculating indices in the generated shader:

program_source:387:223: error: invalid type \'long\' for attribute index expression\nvertex main0_out main0(main0_in in [[stage_in]], constant PushConsts& pushConstants [[buffer(0)]], const device sGpuCache& v_239 [[buffer(1)]], constant Locals& v_678 [[buffer(2)]], texture2d<float> sRenderTasks [[texture(4294967295)]], texture2d<float> sTransformPalette [[texture(4294967295)]])

The related line from the generated shader: https://gist.github.com/imiklos/5451905cefddc6c6a28f2017eed0891a#file-generated_metal-glsl-L387

zakorgy commented 4 years ago

The crash originates from here https://github.com/gfx-rs/gfx/commit/1d2a27f7d35b0265853d17d7ff64cfebee2c3fdb .spirv_crosswas updated from 0.14 to 0.15 most probably that causes the problem.

kvark commented 4 years ago

It turned out to be my mistake, not spirv_cross regression. Now fixed in https://github.com/gfx-rs/gfx/commit/23a49e9504c1bf9c04c14b17da1fecb98276fea2 . The Metal backend is already published - https://crates.io/crates/gfx-backend-metal/0.3.3 Please update your Cargo toml/lock

kvark commented 4 years ago

oh wow

zakorgy commented 4 years ago

Thanks @kvark for the Metal update!