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

Direct2d exploration #66

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Direct2d exploration #66

wants to merge 11 commits into from

Conversation

sudara
Copy link
Owner

@sudara sudara commented Jun 18, 2024

Hey @mattgonzalez

This is what I did:

  • Copied some code in from the module branch of your project
  • Added a Direct2D implementation
  • Enabled running tests on the implementation
  • Try to get single channel to work

Feel free to jump in and run the tests target. You can open this project in CLion (or whatever will build cmake projects) or run cmake from the command line to build and run the tests.

These are the problems/thoughts so far:

  • As far as I can tell, there's nothing too "expensive" with getting the adapter and device context, it's basically asking around for pointers, is this correct? — can we do this for every call to the blur, or is it expensive and should be cached (I've made it static here, but i think that will cause problems in reality)
  • It doesn't compile. The juce::DxgiAdapter::Ptr adapter; line is unhappy for some reason, when compiling I get an error (cannot convert argument 1 from const int to juce::DxgiAdapter::Ptr).
  • Source and Destination — for single channel images, they are usually rendered paths for drop shadow usage. That means that the image being fed to us could be the source and destination — does Direct2D allow this?
  • Single Channel images — maybe Direct2D needs the image to be converted to ARGB to run the effect, not sure?
  • For now we can proceed with assuming all incoming images are direct2d backed — I think. IMO if we want to check for that, it's before the direct2d algorithm is chosen, so we don't need a bunch of conditional logic for that in the implementation.

Note there's some oddities about running CMake from the root of a JUCE module so you'll need to NOT use Ninja (if you do, you'll run into an infinite loop).

Warning If you do get the tests running, it might crap a dozen images onto your desktop. That's how I've been doing manual inspection. I uhh.... should modify to put them in a subfolder.

@mattgonzalez
Copy link
Collaborator

These are the problems/thoughts so far:

* [ ]  As far as I can tell, there's nothing too "expensive" with getting the adapter and device context, it's basically asking around for pointers, is this correct? — can we do this for every call to the blur, or is it expensive and should be cached (I've made it static here, but i think that will cause problems in reality)

The adapters are already stored by JUCE. We'll probably want to cache the device context.

* [ ]  It doesn't compile. The `juce::DxgiAdapter::Ptr adapter;` line is unhappy for some reason, when compiling I get an error (`cannot convert argument 1 from const int to juce::DxgiAdapter::Ptr`).

I'll take a look.

* [ ]  Source and Destination — for single channel images, they are usually rendered paths for drop shadow usage. That means that the image being fed to us _could_ be the source and destination — does Direct2D allow this?

Fairly certain that for D2D effects the source & destination need to be different. Could just copy the source back to the destination when we're done and keep a temporary scratchpad image for the output.

* [ ]  Single Channel images — maybe Direct2D needs the image to be converted to ARGB to run the effect, not sure?

I need to figure out how to convert image formats in the GPU instead of on the CPU.

* [ ]  For now we can proceed with assuming all incoming images are direct2d backed — I _think_. IMO if we want to check for that, it's _before_ the direct2d algorithm is chosen, so we don't need a bunch of conditional logic for that in the implementation.

OK

Note there's some oddities about running CMake from the root of a JUCE module so you'll need to NOT use Ninja (if you do, you'll run into an infinite loop).

Warning If you do get the tests running, it might crap a dozen images onto your desktop. That's how I've been doing manual inspection. I uhh.... should modify to put them in a subfolder.

@mattgonzalez
Copy link
Collaborator

  • It doesn't compile. The juce::DxgiAdapter::Ptr adapter; line is unhappy for some reason, when compiling I get an error (cannot convert argument 1 from const int to juce::DxgiAdapter::Ptr).

This may be due to mixing the JUCE headers with the system headers. You may have better success if you can include the Windows headers first before any JUCE headers.

@sudara
Copy link
Owner Author

sudara commented Jun 19, 2024

Thanks for the comments!

No matter the order of the includes, I can't get references to DxgiAdapter and friends to compile.

For now, I've simplified and just trying to do everything in 1 function.

My suspicion is part of the problem comes from trying to "reach into" the juce_graphics module internals. When one includes a juce module, one is supposed to just include the top level header like "juce_graphics/juce_graphics.h" which then goes and includes everything it needs to be happy — it usually causes problems to root around and try and include random files from modules. It also feels a bit wrong to be including windows.h and so on...

BUT (big BUT!) I'm out of my depth here in Windows-land, so I have no idea what half of this stuff is :D

I guess only JUCE module cpp implementation files are including things like juceDirect2Image_windows.h...I wonder if I should try and work with header/implementation instead of just header...

@sudara
Copy link
Owner Author

sudara commented Jun 25, 2024

Got single channel working, with a temporary image as the destination which then copies back to the source.

Benchmarks are very promising, showing improvements that is an order of magnitude faster on larger image sizes in Release and improved performance in Debug (this is important).

benchmark name                       samples       iterations    est run time
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
Gin (reference implementation)                 100             1    145.294 ms
                                        1.51957 ms    1.48234 ms    1.57098 ms
                                        221.163 us    174.718 us    308.974 us

Naive (Circular Buffer)                        100             1    208.114 ms
                                        2.07789 ms    2.03205 ms    2.13284 ms
                                        255.282 us    217.386 us    347.802 us

JUCE FloatVectorOperations                     100             1    163.628 ms
                                         1.6083 ms    1.55488 ms    1.67932 ms
                                        311.824 us     253.98 us    417.637 us

Melatonin                                      100             1    160.026 ms
                                        1.59018 ms    1.53473 ms    1.67147 ms
                                        338.816 us    260.155 us    472.774 us

Direct2D                                       100             1    14.4098 ms
                                         154.22 us    146.595 us    168.319 us
                                        50.9791 us    32.0538 us    82.2537 us

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants