-
Notifications
You must be signed in to change notification settings - Fork 22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an autogenerated code review.
Checker summary (by rust_clippy):
The tool has found 0 warnings, 30 errors.
The .monocodus
config not found in your repo. Default config is used.
Check config documentation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an autogenerated code review.
Checker summary (by rust_clippy):
The tool has found 0 warnings, 30 errors.
The .monocodus
config not found in your repo. Default config is used.
Check config documentation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an autogenerated code review.
Checker summary (by rust_clippy):
The tool has found 0 warnings, 30 errors.
The .monocodus
config not found in your repo. Default config is used.
Check config documentation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an autogenerated code review.
Checker summary (by rust_clippy):
The tool has found 0 warnings, 30 errors.
The .monocodus
config not found in your repo. Default config is used.
Check config documentation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an autogenerated code review.
Checker summary (by rust_clippy):
The tool has found 0 warnings, 30 errors.
The .monocodus
config not found in your repo. Default config is used.
Check config documentation here
self.resources = Some(resources); | ||
} | ||
|
||
pub fn load_assets(&mut self) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't required to demonstrate the composition. Just clearing the screen (or doing multiple clears of sub-rectangles) would do the trick, and would end up with less code here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this!
I have no idea what code I can remove from this yet. Or how to "clear sub-rectangles"? Code in this example I basically copy & pasted blindly from gfx and microsoft's DirectX examples until I got something visible on screen. And I still have no idea what I can just "take away" and have working example.
@@ -114,6 +118,57 @@ pub struct SwapchainDesc { | |||
} | |||
|
|||
impl Factory2 { | |||
pub fn create_swapchain_for_composition_hwnd( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems reasonable!
Is the "_hwnd" suffix intentional here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was intentional, because it also calls IDCompositionTarget::CreateTargetForHwnd
with given hwnd. However the way this d3d12-rs crate is structured, I probably shouldn't be calling multiple things in one function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think I will refactor this so that it's just create_swapchain_for_composition
, and it doesn't call other functions. Then I will do comp.rs
where I move this other stuff such as creating wrappers for these:
// Create IDCompositionTarget for the window
let mut comp_target = CompTarget::null();
unsafe { comp_device.CreateTargetForHwnd(hwnd as _, 1, comp_target.mut_void() as _) };
// Create IDCompositionVisual
let mut comp_visual = CompVisual::null();
unsafe { comp_device.CreateVisual(comp_visual.mut_void() as _) };
// Set swap_chain and the root visual and commit
unsafe {
comp_visual.SetContent(swap_chain.as_mut_ptr() as _);
comp_target.SetRoot(comp_visual.as_mut_ptr() as _);
comp_device.Commit();
}
I move each of these in own function in comp.rs
and I probably need to do libloading
feature for the DCompositionCreateDevice
function so that it need not be linked.
Not sure , but I think that code I wrote leaks resources: IDCompositionTarget
and IDCompositionVisual
because WeakPtr
doesn't call Release on drop.
I'm used to this kind of implementation for ComPtrs, if we don't call Release
on drop, it becomes a bit cumbersome to track the life time IMO.
This is just preliminary PR for #29, not to be merged yet.
I have to probably refactor this a bit, but it's now initial proof of concept.
It's very possible that this d3d12 repo doesn't need to have examples, but I had to do one to test my code before trying to hack the bigger repo
gfx
.Includes a simple
examples/composition.rs
that renders D3D window with test alpha channeled transparent background color to see it works. Run example with: