fix(core): Flush pending writes before mapAsync, if needed

[andyleiserson]

Fixes #5173

This implements the strategy @teoxoy suggested in bug 1994733, comment 6: if mapAsync finds that PendingWrites holds writes for the buffer being mapped, then PendingWrites is flushed/submitted before mapping the buffer.

This strategy never makes Queue::writeBuffer or unmap perform a submission. A submission only happens if/when mapAsync is called for a buffer with pending writes. This has the nice property that mapAsync was already an operation that might have to wait on the queue, although this change broadens the circumstances when that will happen and may increase the amount of queued work that mapAsync has to wait on. (More sophisticated approaches would be to only flush the pending writes for the buffer in question, rather than all of them, or to proactively flush pending writes if the queue is idle, to reduce the chance that mapAsync ends up waiting on them.)

In terms of code changes, this didn't end up being that bad, but I'd definitely consider it a risky change and I am not sure how best to shake out any potential issues.

There is a long-standing TODO in mapAsync about possibly needing a barrier. I briefly considered trying to address that as part of that change, but I think it's a separable concern, and comes with its own set of performance risks that it is better not to lump together with this change.

Testing
Adds a directed test and some CTS tests.

Squash or Rebase? Squash

Checklist

• Need to add a CHANGELOG.md entry.

[teoxoy]

Good stuff!

[teoxoy]

I think it was ok because we were not using the raw hal buffer for any operation but we were chaning state on the core buffer so, maybe we could have gotten into an unexpected situation.

[teoxoy]

The spec allows the internal variant to only be returned by pipeline creation.

[teoxoy]

I think we should either panic here or add a new submit function to make it impossible for these variants to be returned.

[teoxoy]

Refactoring the submit function to be used by a new submit function that doesn't return these variants seems a lot more tricky.

[teoxoy]

Maybe it's possible to take out this block and call it from map_async.

wgpu/wgpu-core/src/device/queue.rs

Lines 1375 to 1470 in a58f51a

	let mut pending_writes = self.pending_writes.lock();
	{
	used_surface_textures.set_size(self.device.tracker_indices.textures.size());
	for texture in pending_writes.dst_textures.values() {
	match texture.try_inner(&snatch_guard) {
	Ok(TextureInner::Native { .. }) => {}
	Ok(TextureInner::Surface { .. }) => {
	// Compare the Arcs by pointer as Textures don't implement Eq
	submit_surface_textures_owned
	.insert(Arc::as_ptr(texture), texture.clone());
	unsafe {
	used_surface_textures
	.merge_single(texture, None, wgt::TextureUses::PRESENT)
	.unwrap()
	};
	}
	// The texture must not have been destroyed when its usage here was
	// encoded. If it was destroyed after that, then it was transferred
	// to `pending_writes.temp_resources` at the time of destruction, so
	// we are still okay to use it.
	Err(DestroyedResourceError(_)) => {}
	}
	}
	if !used_surface_textures.is_empty() {
	let mut trackers = self.device.trackers.lock();
	let texture_barriers = trackers
	.textures
	.set_from_usage_scope_and_drain_transitions(
	&used_surface_textures,
	&snatch_guard,
	)
	.collect::<Vec<_>>();
	unsafe {
	pending_writes
	.command_encoder
	.transition_textures(&texture_barriers);
	};
	}
	}
	match pending_writes.pre_submit(&self.device.command_allocator, &self.device, self) {
	Ok(Some(pending_execution)) => {
	active_executions.insert(0, pending_execution);
	}
	Ok(None) => {}
	Err(e) => break 'error Err(e.into()),
	}
	let hal_command_buffers = active_executions
	.iter()
	.flat_map(|e| e.inner.list.iter().map(|b| b.as_ref()))
	.collect::<Vec<_>>();
	{
	let mut submit_surface_textures =
	SmallVec::<[&dyn hal::DynSurfaceTexture; 2]>::with_capacity(
	submit_surface_textures_owned.len(),
	);
	for texture in submit_surface_textures_owned.values() {
	let raw = match texture.inner.get(&snatch_guard) {
	Some(TextureInner::Surface { raw, .. }) => raw.as_ref(),
	_ => unreachable!(),
	};
	submit_surface_textures.push(raw);
	}
	if let Err(e) = unsafe {
	self.raw().submit(
	&hal_command_buffers,
	&submit_surface_textures,
	(fence.as_mut(), submit_index),
	)
	}
	.map_err(|e| self.device.handle_hal_error(e))
	{
	break 'error Err(e.into());
	}
	drop(command_index_guard);
	// Advance the successful submission index.
	self.device
	.last_successful_submission_index
	.fetch_max(submit_index, Ordering::SeqCst);
	}
	profiling::scope!("cleanup");
	// this will register the new submission to the life time tracker
	self.lock_life()
	.track_submission(submit_index, active_executions);
	drop(pending_writes);

Below it we call device.maintain to cleanup resources of previous submissions but I don't think we should do that in map_async.