Query API and basic support on Metal #2334

kvark · 2018-08-20T17:15:03Z

Fixes #1638 (for the most part)
PR checklist:

make succeeds (on *nix)
make reftests succeeds
tested examples with the following backends:
rustfmt run on changed code

kvark · 2018-08-20T20:01:53Z

Looks like we are blocked on ash-rs/ash#100 for Vulkan :(
cc @MaikKlein

kvark · 2018-08-21T20:12:49Z

Current status: 136 / 150 occlusion tests passing
Known 2 issues:

sometimes getting the query results synchronously gets stuck
in some cases copying results with availability doesn't produce correct results

kvark · 2018-08-21T22:08:19Z

Vulkan is blocked on ash-rs/ash#105 now

kvark · 2018-08-22T03:35:28Z

Update on Metal: all the known issues are resolved. We pass 150/150 tests without failures or hangs 🎉
The PR is blocked on reviews and getting Vulkan backend to compile. In the worst case we'll comment out the Vulkan parts that are blocking.

msiglreith

HAL changes LGTM

grovesNL · 2018-08-22T18:50:39Z

src/hal/src/device.rs


    /// Destroy a query pool object
    fn destroy_query_pool(&self, pool: B::QueryPool);

+    /// Get query pool results into the specified CPU memory.
+    /// Returns `Ok(false)` if the resutls are not ready yet and neither of `WAIT` or `PARTIAL` flags are set.


resutls -> results

grovesNL · 2018-08-22T19:00:12Z

src/backend/vulkan/src/command.rs

    ) {
        let mut flags = vk::QueryControlFlags::empty();
-        if control.contains(query::QueryControl::PRECISE) {
+        if control.contains(query::ControlFlags::PRECISE) {


map_query_control_flags instead?

grovesNL · 2018-08-22T19:10:57Z

src/backend/metal/src/device.rs

+                let is_ready = if flags.contains(query::ResultFlags::WAIT) {
+                    let mut guard = visibility.allocator.lock();
+                    while !visibility.are_available(pool_range.start, &queries) {
+                        println!("query: not available"); //TEMP


Is this left here intentionally?

grovesNL · 2018-08-22T19:16:14Z

src/backend/metal/src/command.rs

+                    let queries = self.active_visibility_queries
+                        .drain(..)
+                        .collect::<SmallVec<[_; BLOCK_BUCKET]>>();
+                    Some((Arc::clone(&self.shared), queries))


Why do we need to cloneself.shared?

we clone the Arc, so that we can use the visibility shared structures in the callback

Oh, I didn't notice the callback block. Sounds good

grovesNL · 2018-08-22T19:18:42Z

src/backend/metal/src/command.rs

+                                    size: size_meta,
+                                },
+                            };
+                            // An extra paddig is requred if the client expects 64bits availability without a wait


paddig -> padding

kvark · 2018-08-22T19:23:44Z

Thanks @grovesNL ! comments are addressed

grovesNL · 2018-08-23T01:12:23Z

src/backend/metal/src/command.rs

+
+                        let (com_avail, com_pad) = if flags.contains(query::ResultFlags::WITH_AVAILABILITY | query::ResultFlags::WAIT) {
+                            // Technically waiting is a no-op on a single queue. However,
+                            // the client expects the avaiability to be set regardless.


avaiability -> availability

kvark · 2018-08-23T01:58:30Z

Landing, as per approval on gitter.
bors r=grovesNL

2334: Query API and basic support on Metal r=grovesNL a=kvark Fixes #1638 (for the most part) PR checklist: - [x] `make` succeeds (on *nix) - [x] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

bors · 2018-08-23T02:16:54Z

Build failed

continuous-integration/travis-ci/push

kvark · 2018-08-23T02:28:45Z

Bors retry

…

On Aug 22, 2018, at 22:16, bors[bot] ***@***.***> wrote: Build failed continuous-integration/travis-ci/push — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

2334: Query API and basic support on Metal r=grovesNL a=kvark Fixes #1638 (for the most part) PR checklist: - [x] `make` succeeds (on *nix) - [x] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

bors · 2018-08-23T02:53:29Z

Build failed

continuous-integration/travis-ci/push

kvark · 2018-08-23T03:24:52Z

Bors retry

…

On Aug 22, 2018, at 22:53, bors[bot] ***@***.***> wrote: Build failed continuous-integration/travis-ci/push — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

kvark · 2018-08-23T04:08:06Z

Bors r=grovesNL

…

On Aug 22, 2018, at 22:53, bors[bot] ***@***.***> wrote: Build failed continuous-integration/travis-ci/push — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

bors · 2018-08-23T04:08:07Z

Not awaiting review

2334: Query API and basic support on Metal r=grovesNL a=kvark Fixes #1638 (for the most part) PR checklist: - [x] `make` succeeds (on *nix) - [x] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

kvark · 2018-08-23T04:17:54Z

Oh yes you are, silly bors!

…

On Aug 23, 2018, at 00:08, bors[bot] ***@***.***> wrote: Not awaiting review — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

bors · 2018-08-23T05:09:46Z

Build succeeded

kvark force-pushed the query branch from 554d24b to 946cd9c Compare August 20, 2018 20:01

kvark added 4 commits August 21, 2018 16:08

[mtl] query support - part 1

98c2273

Add missing query commands and result flags

a876fa7

Backends update for the new Query API

d107ef4

[mtl] basic query getting of results

3ea3e10

kvark force-pushed the query branch from 946cd9c to f855c90 Compare August 21, 2018 20:09

kvark force-pushed the query branch from f855c90 to ba07484 Compare August 21, 2018 20:44

kvark changed the title ~~[WIP] Query API and basic support on Metal~~ Query API and basic support on Metal Aug 21, 2018

[mtl] query availability info

b99cd1e

kvark force-pushed the query branch from ba07484 to b99cd1e Compare August 22, 2018 03:33

kvark mentioned this pull request Aug 22, 2018

Vulkan Portability code path on MacOS with MVK extensions RPCS3/rpcs3#4996

Merged

msiglreith reviewed Aug 22, 2018

View reviewed changes

kvark force-pushed the query branch from cb2ee9e to 194e97a Compare August 22, 2018 18:41

grovesNL reviewed Aug 22, 2018

View reviewed changes

kvark force-pushed the query branch from 194e97a to def110e Compare August 22, 2018 19:23

grovesNL reviewed Aug 23, 2018

View reviewed changes

Vulkan and dx12 build fixes

255959b

kvark force-pushed the query branch from def110e to 255959b Compare August 23, 2018 01:56

bors bot merged commit 255959b into gfx-rs:master Aug 23, 2018

kvark deleted the query branch August 23, 2018 14:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Query API and basic support on Metal #2334

Query API and basic support on Metal #2334

kvark commented Aug 20, 2018 •

edited

Loading

kvark commented Aug 20, 2018

kvark commented Aug 21, 2018 •

edited

Loading

kvark commented Aug 21, 2018

kvark commented Aug 22, 2018

msiglreith left a comment

grovesNL Aug 22, 2018

grovesNL Aug 22, 2018

grovesNL Aug 22, 2018

grovesNL Aug 22, 2018

kvark Aug 22, 2018

grovesNL Aug 22, 2018

grovesNL Aug 22, 2018

kvark commented Aug 22, 2018

grovesNL Aug 23, 2018

kvark commented Aug 23, 2018

bors bot commented Aug 23, 2018

kvark commented Aug 23, 2018 via email

bors bot commented Aug 23, 2018

kvark commented Aug 23, 2018 via email

kvark commented Aug 23, 2018 via email

bors bot commented Aug 23, 2018

kvark commented Aug 23, 2018 via email

bors bot commented Aug 23, 2018

Query API and basic support on Metal #2334

Query API and basic support on Metal #2334

Conversation

kvark commented Aug 20, 2018 • edited Loading

kvark commented Aug 20, 2018

kvark commented Aug 21, 2018 • edited Loading

kvark commented Aug 21, 2018

kvark commented Aug 22, 2018

msiglreith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvark commented Aug 22, 2018

Choose a reason for hiding this comment

kvark commented Aug 23, 2018

bors bot commented Aug 23, 2018

Build failed

kvark commented Aug 23, 2018 via email

bors bot commented Aug 23, 2018

Build failed

kvark commented Aug 23, 2018 via email

kvark commented Aug 23, 2018 via email

bors bot commented Aug 23, 2018

kvark commented Aug 23, 2018 via email

bors bot commented Aug 23, 2018

Build succeeded

kvark commented Aug 20, 2018 •

edited

Loading

kvark commented Aug 21, 2018 •

edited

Loading