Dithering is a method used to prevent color banding (https://www.anisopteragames.com/how-to-fix-color-banding-with-dithering/)

In the current code, dithering is only applied to the bloom layer, but the simulation being composed of smooth gradients anyway, it should probably be applied even if bloom is disabled, on the final frame.

context: #20 (comment)

When set settings.grfx.enableDebug = true; the sample will report following validation error:

*** VULKAN VALIDATION ERROR MESSAGE ***
Severity : ERROR
Type     : [VALIDATION]
Objects  : [0]: <UNNAMED OBJECT>
Message  : Validation Error: [ VUID-vkAcquireNextImageKHR-semaphore-01286 ] Object 0: handle = 0x59ffe0000000003d, type = VK_OBJECT_TYPE_SEMAPHORE; | MessageID = 0xe9e4b2a9 | vkAcquireNextImageKHR: Semaphore must not be currently signaled. The Vulkan spec states: If semaphore is not VK_NULL_HANDLE it must be unsignaled (https://vulkan.lunarg.com/doc/view/1.3.236.0/windows/1.3-extensions/vkspec.html#VUID-vkAcquireNextImageKHR-semaphore-01286)

We should document the preferred style for CMake formatting, since there's currently no enforcement and the style changes across files.

From #20 (comment)

In order to load .glb format files, we will need to be able to create textures from a buffer, rather than a file.

Right now, the chosen sample is hard-coded in the manifest. We should find a way to reuse those. Probably with a manifest template, generating the real manifest at build time.
Or maybe we should pack all samples in one APK and have a way to select the one we start (each sample being an activity entrypoint?)

Year in copyright license blobs are out of date.

There is a problem on how we generate CMake rules for shader targets, specifically on Windows.

We use MAIN_DEPENDENCY to specify the input shader file (<shader_name>.hlsl) when we create custom commands to compiler shaders with DXC (add_custom_command). We use MAIN_DEPENDENCY instead of DEPENDS because the former allows Visual Studio to avoid trying to compile the HLSL file with its built-in compiler (FXC). However, due to Visual Studio limitations, it is not possible to have multiple add_custom_command rules with the same MAIN_DEPENDENCY file. We do that when we compile the same shader for multiple stages. This does work in the sense that CMake generation succeeds, but it creates implicit multiple targets that cause Visual Studio to try to compile the same file multiple times in parallel causing sporadic build failures (in case the rules run in parallel).

To reproduce, try generating a VS solution and building a shader target with multiple stages.

I will add more specific information soon.

Some other samples (https://github.com/SaschaWillems/Vulkan/blob/master/examples/pushconstants/pushconstants.cpp) using push constants are able to run on SwiftShader. The size used is below the device limit advertised, so that shouldn't be a problem.

The contributing page advises contributors to make sure that the code "follows the Google C++ Style Guide." We currently don't follow the Google style guide. We should probably remove the reference and document the style guide we want to follow, and update .clang-format if necessary

Some samples rely on keyboard/mouse inputs. This is not available on Android, so we should find a way to add input support (accel, touch?)

--curl 0.0
--curl 30000.0

Both look approximately similar.

When running those benchmarks, the first frame could be impacted by external factors, like resource loading, lazy linking, etc..
It could be useful to add a --warmup-frame-count option, not taking those first N frames into account for benchmark metrics.

GLTF file format has this concept of buffer views.
https://kcoley.github.io/glTF/specification/2.0/figures/gltfOverview-2.0.0a.png

(assuming vulkan concepts)
The idea is that you get a single memory allocation for all the mesh data (indices, vertices, attributes), and load it as once.
Then, you setup your buffers on top to access defined sub-ranges indices and vertex data.

Right now, the Geometry & mesh object is setup with the following API:

GetIndexBuffer()
GetVertexBuffer(uint32_t index);

This means loading a model requires doing the following:

geometry = parse_input_file("my_model.obj");
mesh = create_gpu_object();
staging_buffer = create_staging_buffer();
cpu_to_gpu(geometry.index_buffer(), staging_buffer);
gpu_to_gpu(staging_buffer, mesh.index_buffer());
cpu_to_gpu(geometry.vertex_buffer(0), staging_buffer);
gpu_to_gpu(staging_buffer, mesh.vertex_buffer(0));
cpu_to_gpu(geometry.vertex_buffer(1), staging_buffer);
gpu_to_gpu(staging_buffer, mesh.vertex_buffer(1));

The API might benefit from adding this buffer_view concept to bigwheels. This way, we would have only 1 buffer to copy for such models.

Context:

#46
#46 (comment)
#46 (comment)

Some changes to the fishtornado samples have been made to avoid a bug on Pixel 6. We should revisit this to really understand the issue, and fix it.

Step 1: enable swiftshader
https://github.com/google/bigwheels/blob/main/docs/software_rendering.md

Step 2: run a sample
for example: 05_cube_textured
or to see more clearly: run vk_27_mipmap_demo, select a different mip level (> 0) to be generated using GPU

Background:
Mipmap creation code in https://github.com/google/bigwheels/blob/main/src/ppx/graphics_util.cpp#L273

Platform: Windows

Description:
When I run Debug applications/benchmarks and try to minimize the window, it crashes. I've noticed this consistently happens for vk_* binaries but not for dx12_*. Able to reproduce for the following:

• vk_01_triangle.exe
• vk_17_primitives.exe
• vk_fishtornado.exe
• vk_graphics_pipeline.exe
• ...

Error:

*** PPX ASSERT ***
Message   : vkQueuePresentKHR failed: VK_ERROR_OUT_OF_DATE_KHR
Condition : false
Function  : ppx::grfx::vk::Swapchain::PresentInternal
Location  : C:\src\git\bigwheels\src\ppx\grfx\vk\vk_swapchain.cpp : 619

Assertion failed: false, file C:\src\git\bigwheels\src\ppx\grfx\vk\vk_swapchain.cpp, line 619

I recently mis-spelled a command-line argument that I had added to my application. This produced no error output from the execution. The argument was silently ignored.

If an argument is not handled by the built-in command-argument parser nor the application, an error should be emitted and execution aborted.

From an internal Android linter: The app manifest must specify an empty taskAffinity on the <application>. on AndroidMainfest.xml:5.

See the warning in the internal CL # 524944258

Currently all extensions that can be enabled are enabled.

context: https://github.com/google/bigwheels/pull/20/files/66100b1682d1ce7b4566417f90bcb1f7494c5eb3#r1060646881

When changing the aspect ratio, the simulation aspect changes because the radius of slats is only taken into account for wide screens, not tall ones.

This should be fixable by multiplying radius by aspect in all cases.

CreateImageFromCompressedImage seems to assume that compressed images only have a single mipmap level. .dds files can store multiple mipmap levels however, and we want to use that feature to speed up load times.

Address the problems discussed in #20 (comment)_

This breaks the bloom

changing it to REPEAT fixes it (should only be applied for this texture, not the speed one and others, otherwise speed will leak around the edges.

The folowing projects do not have their corresponding .h/.cpp files in the project's tree in Visual Studio:

• 01_triange
• 02_triangle_spinning
• 03_square_textured
• 04_cube

There might be more but these are the ones that I saw.

For samples that are texture heavy, move the mipmap generation offline to reduce the startup time of the applications.

DearImGUI seems to have support for Android. The initial porting to Android disabled it, but we should find a way to enable it back.

A new CI workflow should be added to test that BigWheels for Android builds correctly.

Usually when the orientation is changed on Android, the activity is destroyed and restored.
Need to figure out how this works with native bigwheels.
Same remark for foldable, or any kind of screen aspect change.
Probably low-priority, but an enhancement we could bring.

It seems like fishtornado(xr) and 28_gltf projects currently don't have a README. It might be useful to add, particularly to document additional command line arguments such as the ones added in #153

#46 brought Android support, but cluttered the project. We should be able to remove many optional things (custom theme, proguard, etc)

Current geometry (and tri_mesh) support appending new vertices/attributes, etc.
This is useful when building a mesh on the fly, or when parsing some stream.

• Current geometry API supports both linear, or interleaved data layout, and provide multiple functions to append new attributes (doesn't support deletion, appending is limited to linear layout for some function).
• Current Geometry::Buffer allow mutation through "append".

If #28 is accepted, buffer will be decoupled from the allocated memory block.
Allowing import of GLTF meshes with multiple buffers using the same underlying allocation block, while still supporting those mutable geometry/buffers APIs could become complex as appending to a buffer will imply changing neighbor buffer offsets, and underlying allocation size.

For this reason, I'd suggest having 2 kind of buffer/geometry:

• mutable geometry/buffers (as we have today). Using a 1:1 mapping for buffer/geometry.
• static geometry, allowing a single memory allocation for multiple buffers.

An alternative solution would be to keep the current geometry class, and handle it like the interleaved vs linear layout: return a failure in some cases. But it might be harder to maintain, and also become harder to use as we would need to know the layout, and how an object was created to use it correctly.

The initial PR relied on a define to either create a simple main, or create multiple functions for Android.
We should be able to solve that at link-time, in a cleaner way.

It's currently not possible to change the size of the window running the simulation.

Initial Android support (#46) doesn't have this. We should enable it back on this platform.

The function CreateIBLTextureFromFile doesn't work on Android, as android cannot use fstream, and should go through the asset manager.

Issues: https://github.com/google/bigwheels/pull/117/files/14b03b6294feec086df7ef8db4fffe6f29ea530a#diff-d1e3258e70bf12dc8481423798d4de28900ca1e9b015bd1eebe3e1bd4e50d860R1158

From #20 (comment)

Dispatching a compute shader with large dimensions and using numthreads(1,1,1) is very bad for performance, due to reduced occupancy. Most common graphics cards use a warp size of either 32 (NVIDIA) or 64 (AMD), meaning using numthreads(1,1,1) wastes 31 or 63 threads. The GPU cannot utilize the wasted threads within a warp, meaning you're getting something like at most 1/32 of the GPU performance in this compute-heavy application.

You'd want to use something like
numthreads(8,8,1) and then dispatchSize(ppx::float3(ceil(dr.mOutput->GetWidth() / 8), ceil(dr.mOutput->GetHeight() / 8), 1). You also need to modify the shaders to compute XY coordinates slightly differently.

Platform: Windows
NVIDIA Quadro P1000
Driver version: 30.0.15.1109

Description: vk_graphics_pipeline.exe runs fine but dx12_graphics_pipeline.exe appears to crash on startup

Error message:

*** PPX ASSERT ***
Message   : Descriptor binding type is not pushable in Direct3D 12
Condition : false
Function  : ppx::grfx::dx12::PipelineInterface::CreateApiObjects
Location  : C:\src\git\bigwheels\src\ppx\grfx\dx12\dx12_pipeline.cpp : 298

Assertion failed: false, file C:\src\git\bigwheels\src\ppx\grfx\dx12\dx12_pipeline.cpp, line 298

Should get the android window size instead of a hardcoded value of 720. IMGUI as a result has a weird aspect ratio

I can't remember why the name GeometryOptions was use initially. It should really be called GeometryCreateInfo since all the Geometry::Create methods use the var name createInfo for any parameter of type GeometryOptions.

One of the main goals of this refactoring is to allow the simulation to use any given command buffer, device and resolution.

• Implement a static Create function to initialize an instance of the simulator with error handling for resource creation.
• Use an app-provided command buffer instead of having a fixed one.
• Allow access to intermediate textures.

The shader display.hlsl (

Does not process android input at all

These are unimplemented, but would be useful for compute shader experiments.

DX12: https://github.com/google/bigwheels/blob/main/src/ppx/grfx/dx12/dx12_descriptor.cpp#L315

DX11: https://github.com/google/bigwheels/blob/main/src/ppx/grfx/dx11/dx11_descriptor.cpp#L260

Can we add an arg (or maybe a different function) that allows us to specify which project folder a sample or benchmark should go into? Currently both samples and benchmarks show up under samples in Visual Studio.

When working on the 28_gltf sample, I loaded Sponza, which allocated 630+ descriptors of each type (limit was set to 512).
Turns out some drivers are fine with that, other will fail, but then allow as soon as you bump the size of any descriptor type.
(https://gitlab.freedesktop.org/mesa/mesa/-/issues/8411)
Also, vulkan validation layers don't catch this one.

It might be useful for bigwheels to check the pool allocation limits, and fail when we try to allocate more than we asked for, so we don't end up with a changing behavior depending on the hardware/API.

clang-format does not play nice with HLSL: it works fine for the code within functions but messes up the struct definitions especially when semantics are specified.

We should either find a way to work around clang-format to ignore the definitions and at least format the code, or specify a HLSL style guide that should be followed manually.

The buffers in Geometry currently make a bunch of allocations whne appending data. If the buffer doesn't have a max element count, it should grow by 1.5x or 2x the current capacity as opposed to the size of the element.

Hello!

Merging #148 broke HEAD because the CI run before some other PR was merged, and this code "conflicted" with the other.
A solution to avoid those issues is to require up-to-date before merging.

This means we won't be able to merge multiple PRs at once, but makes merging safer.
Is anyone against this change?

@apazylbe found out this:

./gradlew assembleMobileTriangleDebug -PDXC_PATH=$(realpath ~/projects/DirectXShaderCompiler/build/bin/dxc)
cd build/outputs/apk/mobileTriangle/debug/
unzip bigwheels-mobile-triangle-debug.apk
androguard axml AndroidManifest.xml

Generated XML:

<manifest xmlns:android="http://schemas.android.com/apk/res/android" android:versionCode="1" android:versionName="1.0" android:compileSdkVersion="32" android:compileSdkVersionCodename="12" package="com.google.bigwheels.debug" platformBuildVersionCode="32" platformBuildVersionName="12">
  <uses-sdk android:minSdkVersion="29" android:targetSdkVersion="32"/>
  <uses-feature android:name="android.hardware.sensor.accelerometer" android:required="true"/>
  <uses-feature android:name="android.hardware.sensor.gyroscope" android:required="true"/>
  <uses-feature android:name="android.hardware.vr.headtracking" android:required="false" android:version="1"/>
  <uses-feature android:name="android.hardware.vr.high_performance" android:required="true"/>
  <uses-feature android:name="android.software.vr.mode" android:required="true"/>
  <meta-data android:name="com.oculus.intent.category.VR" android:value="vr_only"/>
  <meta-data android:name="com.oculus.supportedDevices" android:value="quest|quest2|cambria"/>
  <application android:theme="@7F0F01FA" android:label="@7F0E001C" android:icon="@android:010800B4" android:debuggable="true" android:allowBackup="true" android:extractNativeLibs="false" android:appComponentFactory="androidx.core.app.CoreComponentFactory">
    <activity android:name="com.google.bigwheels.MainActivity" android:exported="true">
      <intent-filter>
        <action android:name="android.intent.action.MAIN"/>
        <category android:name="android.intent.category.LAUNCHER"/>
        <category android:name="com.oculus.intent.category.VR"/>
        <category android:name="org.khronos.openxr.intent.category.IMMERSIVE_HMD"/>
      </intent-filter>
      <meta-data android:name="android.app.lib_name" android:value="@7F0E008C"/>
    </activity>
    <provider android:name="androidx.startup.InitializationProvider" android:exported="false" android:authorities="com.google.bigwheels.debug.androidx-startup">
      <meta-data android:name="androidx.emoji2.text.EmojiCompatInitializer" android:value="androidx.startup"/>
      <meta-data android:name="androidx.lifecycle.ProcessLifecycleInitializer" android:value="androidx.startup"/>
    </provider>
  </application>
</manifest>

Looks like the quest manigest is used, not the default one, as we would expect looking at the build.gradle flavors.

I am trying to run the headless_compute benchmark, but it fails when creating the storage buffer:

     RaiseException�()    Unknown
     _CxxThrowException�()    Unknown
     CDevice::CreatePlacedResource_Worker(struct ID3D12Heap *,unsigned __int64,struct D3D12_RESOURCE_DESC1 const *,enum D3D12_RESOURCE_STATES,struct D3D12_CLEAR_VALUE const *,struct _GUID const &,void * *)    Unknown
     CDevice::CreatePlacedResource(struct ID3D12Heap *,unsigned __int64,struct D3D12_RESOURCE_DESC const *,enum D3D12_RESOURCE_STATES,struct D3D12_CLEAR_VALUE const *,struct _GUID const &,void * *)    Unknown
>    D3D12MA::BlockVector::CreateResource(unsigned __int64 size, unsigned __int64 alignment, const D3D12MA::ALLOCATION_DESC & allocDesc, const D3D12_RESOURCE_DESC & resourceDesc, D3D12_RESOURCE_STATES InitialResourceState, const D3D12_CLEAR_VALUE * pOptimizedClearValue, D3D12MA::Allocation * * ppAllocation, const _GUID & riidResource, void * * ppvResource) Line 8484    C++
     D3D12MA::AllocatorPimpl::CreateResource(const D3D12MA::ALLOCATION_DESC * pAllocDesc, const D3D12_RESOURCE_DESC * pResourceDesc, D3D12_RESOURCE_STATES InitialResourceState, const D3D12_CLEAR_VALUE * pOptimizedClearValue, D3D12MA::Allocation * * ppAllocation, const _GUID & riidResource, void * * ppvResource) Line 6840    C++
     D3D12MA::Allocator::CreateResource(const D3D12MA::ALLOCATION_DESC * pAllocDesc, const D3D12_RESOURCE_DESC * pResourceDesc, D3D12_RESOURCE_STATES InitialResourceState, const D3D12_CLEAR_VALUE * pOptimizedClearValue, D3D12MA::Allocation * * ppAllocation, const _GUID & riidResource, void * * ppvResource) Line 10024    C++
     ppx::grfx::dx12::Buffer::CreateApiObjects(const ppx::grfx::BufferCreateInfo * pCreateInfo) Line 64    C++
     ppx::grfx::CreateDestroyTraits<ppx::grfx::BufferCreateInfo>::Create(const ppx::grfx::BufferCreateInfo * pCreateInfo) Line 236    C++
     ppx::grfx::Buffer::Create(const ppx::grfx::BufferCreateInfo * pCreateInfo) Line 35    C++
     ppx::grfx::Device::CreateObject<ppx::grfx::Buffer,ppx::grfx::BufferCreateInfo,std::vector<ObjPtr<ppx::grfx::Buffer>,std::allocator<ObjPtr<ppx::grfx::Buffer>>>>(const ppx::grfx::BufferCreateInfo * pCreateInfo, std::vector<ObjPtr<ppx::grfx::Buffer>,std::allocator<ObjPtr<ppx::grfx::Buffer>>> & container, ppx::grfx::Buffer * * ppObject) Line 110    C++
     ppx::grfx::Device::CreateBuffer(const ppx::grfx::BufferCreateInfo * pCreateInfo, ppx::grfx::Buffer * * ppBuffer) Line 233    C++
     ProjApp::SetupComputeShaderPass() Line 164    C++
     ProjApp::Setup() Line 138    C++
     ppx::Application::DispatchSetup() Line 1003    C++
     ppx::Application::Run(int argc, char * * argv) Line 1365    C++
     main(int argc, char * * argv) Line 281    C++

The call to hr = m_hAllocator->GetDevice()->CreatePlacedResource is failing with E_INVALIDARG One or more arguments are invalid..