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

Possible workaround for Intel GPU bug with nested structs #1949

Merged
merged 1 commit into from Oct 29, 2017

Conversation

@kvark
Copy link
Member

kvark commented Oct 26, 2017

Potentially fixes #1939

@NiLSPACE would you be able to test? I suspect just as simple cd webrender && cargo run --example yuv should be sufficient. Thanks!


This change is Reviewable

@NiLSPACE
Copy link

NiLSPACE commented Oct 26, 2017

I could try, but I've never compiled Servo before, so I'll have to figure that out first. Once I got that working how can I make sure I get this version of WebRender?

@kvark
Copy link
Member Author

kvark commented Oct 26, 2017

@NiLSPACE I don't think you even need Servo to test this.
You can just do:

git clone https://github.com/kvark/webrender -b glsl
cd webrender/webrender
cargo run --example yuv

Then wait till stuff apears in the window ~ 5-10 sec
If it works, we can confirm that the patch does the trick by doing:

git checkout HEAD^
cargo run --example yuv

This is supposed to fail.

@NiLSPACE
Copy link

NiLSPACE commented Oct 26, 2017

Unfortunately I get lots of linking errors due to the machine type (x86) conflicting with the target (x64) so I'll need to figure that out first. I'll get back to this tomorrow.

@bors-servo
Copy link
Contributor

bors-servo commented Oct 27, 2017

The latest upstream changes (presumably #1947) made this pull request unmergeable. Please resolve the merge conflicts.

@NiLSPACE
Copy link

NiLSPACE commented Oct 28, 2017

I'm a day late, I apologise.

I managed to compile WebRender with the example, but unfortunately it doesn't seem like it worked. This was the output in the console:

    Finished dev [unoptimized + debuginfo] target(s) in 168.66 secs
     Running `D:\Projects\webrender_fixed\target\debug\examples\yuv.exe`
OpenGL version 3.2.0 - Build 22.20.16.4735
Shader resource path: None
Failed to compile shader: "cs_clip_image"
ERROR: 0:1110: 'constructor' :  cannot convert parameter 1 from 'float' to 'structure'
ERROR: 0:1109: 'return' : function return is not matching type:


thread 'main' panicked at 'renderer::deinit not called', src\device.rs:438:8
stack backtrace:
   0: std::sys_common::backtrace::_print
             at C:\projects\rust\src\libstd\sys_common\backtrace.rs:94
   1: std::panicking::default_hook::{{closure}}
             at C:\projects\rust\src\libstd\panicking.rs:380
   2: std::panicking::default_hook
             at C:\projects\rust\src\libstd\panicking.rs:397
   3: std::panicking::rust_panic_with_hook
             at C:\projects\rust\src\libstd\panicking.rs:611
   4: std::panicking::begin_panic<str*>
             at C:\projects\rust\src\libstd\panicking.rs:572
   5: webrender::device::{{impl}}::drop
             at .\src\device.rs:438
   6: core::ptr::drop_in_place<webrender::device::Program>
             at C:\projects\rust\src\libcore\ptr.rs:61
   7: core::ptr::drop_in_place<core::option::Option<webrender::device::Program>>
             at C:\projects\rust\src\libcore\ptr.rs:61
   8: core::ptr::drop_in_place<webrender::renderer::LazilyCompiledShader>
             at C:\projects\rust\src\libcore\ptr.rs:61
   9: webrender::renderer::Renderer::new
             at .\src\renderer.rs:1862
  10: yuv::boilerplate::main_wrapper
             at .\examples\common\boilerplate.rs:135
  11: yuv::main
             at .\examples\yuv.rs:267
  12: panic_unwind::__rust_maybe_catch_panic
             at C:\projects\rust\src\libpanic_unwind\lib.rs:99
  13: std::rt::lang_start
             at C:\projects\rust\src\libstd\rt.rs:54
  14: main
  15: __scrt_common_main_seh
             at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283
  16: BaseThreadInitThunk
error: process didn't exit successfully: `D:\Projects\webrender_fixed\target\debug\examples\yuv.exe` (exit code: 101)
@kvark
Copy link
Member Author

kvark commented Oct 28, 2017

@NiLSPACE could you verify that you are getting the original bug when running WR master branch as opposed to my fix?
If I read this correctly, the fix has actually worked, since your log indicates failure in cs_clip_image, which is initialized after brush_mask, which is what original bug is about. Thus, I suspect brush_mask is now fine, and we just need more fixing.

@kvark
Copy link
Member Author

kvark commented Oct 28, 2017

This one actually looks like https://github.com/servo/webrender/wiki/Driver-issues#1251---returning-with-struct-constructor:

return ClipVertexInfo(layer_pos.xyw, actual_pos, clipped_local_rect);

@kvark kvark force-pushed the kvark:glsl branch 2 times, most recently from 658029b to 822096e Oct 28, 2017
@kvark
Copy link
Member Author

kvark commented Oct 28, 2017

I re-implemented the fix on rebased code and added this other workaround.
@NiLSPACE Please test again! (same instructions)

@NiLSPACE
Copy link

NiLSPACE commented Oct 28, 2017

It's still reporting the same error:

    Finished dev [unoptimized + debuginfo] target(s) in 171.79 secs
     Running `D:\Projects\webrender-fixed\target\debug\examples\yuv.exe`
OpenGL version 3.2.0 - Build 22.20.16.4735
Shader resource path: None
Failed to compile shader: "cs_clip_image"
ERROR: 0:1132: 'constructor' :  cannot convert parameter 1 from 'float' to 'structure'
ERROR: 0:1131: 'return' : function return is not matching type:


thread 'main' panicked at 'renderer::deinit not called', src\device.rs:438:8
stack backtrace:
   0: std::sys_common::backtrace::_print
             at C:\projects\rust\src\libstd\sys_common\backtrace.rs:94
   1: std::panicking::default_hook::{{closure}}
             at C:\projects\rust\src\libstd\panicking.rs:380
   2: std::panicking::default_hook
             at C:\projects\rust\src\libstd\panicking.rs:397
   3: std::panicking::rust_panic_with_hook
             at C:\projects\rust\src\libstd\panicking.rs:611
   4: std::panicking::begin_panic<str*>
             at C:\projects\rust\src\libstd\panicking.rs:572
   5: webrender::device::{{impl}}::drop
             at .\src\device.rs:438
   6: core::ptr::drop_in_place<webrender::device::Program>
             at C:\projects\rust\src\libcore\ptr.rs:61
   7: core::ptr::drop_in_place<core::option::Option<webrender::device::Program>>
             at C:\projects\rust\src\libcore\ptr.rs:61
   8: core::ptr::drop_in_place<webrender::renderer::LazilyCompiledShader>
             at C:\projects\rust\src\libcore\ptr.rs:61
   9: webrender::renderer::Renderer::new
             at .\src\renderer.rs:1862
  10: yuv::boilerplate::main_wrapper
             at .\examples\common\boilerplate.rs:135
  11: yuv::main
             at .\examples\yuv.rs:267
  12: panic_unwind::__rust_maybe_catch_panic
             at C:\projects\rust\src\libpanic_unwind\lib.rs:99
  13: std::rt::lang_start
             at C:\projects\rust\src\libstd\rt.rs:54
  14: main
  15: __scrt_common_main_seh
             at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283
  16: BaseThreadInitThunk
error: process didn't exit successfully: `D:\Projects\webrender-fixed\target\debug\examples\yuv.exe` (exit code: 101)
@kvark kvark force-pushed the kvark:glsl branch from 822096e to ff242e7 Oct 29, 2017
@kvark
Copy link
Member Author

kvark commented Oct 29, 2017

Thanks @NiLSPACE ! This is not exactly the same error - line numbers are different, they correspond to:

ImageMaskData fetch_mask_data(ivec2 address) {
    vec4 data = fetch_from_resource_cache_1_direct(address);
    return ImageMaskData(RectWithSize(data.xy, data.zw));
}

A fix for this function is now also pushed to the glsl branch. Please check again 🤞

@NiLSPACE
Copy link

NiLSPACE commented Oct 29, 2017

It's working without errors or warnings now.

@kvark
Copy link
Member Author

kvark commented Oct 29, 2017

Marvelous!
@glennw this is ready to ship then

@glennw
Copy link
Member

glennw commented Oct 29, 2017

Thanks!

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Oct 29, 2017

📌 Commit ff242e7 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Oct 29, 2017

Testing commit ff242e7 with merge c06f298...

bors-servo added a commit that referenced this pull request Oct 29, 2017
Possible workaround for Intel GPU bug with nested structs

Potentially fixes #1939

@NiLSPACE would you be able to test? I suspect just as simple `cd webrender && cargo run --example yuv` should be sufficient. Thanks!

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1949)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 29, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: glennw
Pushing c06f298 to master...

@bors-servo bors-servo merged commit ff242e7 into servo:master Oct 29, 2017
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@kvark kvark deleted the kvark:glsl branch Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.