KhronosGroup / Vulkan-Samples

One stop solution for all Vulkan samples
Apache License 2.0
4.34k stars 648 forks source link

Remove RTP feature request from ray_queries #908

Closed cmannett85-arm closed 9 months ago

cmannett85-arm commented 9 months ago

Description

The ray_queries sample requests the RTP feature support from the GPU, although I can't see why this is done as the RQ extension has no dependency on the RTP extension, and RTP isn't used in the sample. Unsurprisingly removing it has no effect on the devices I've tried it on.

This isn't an issue on desktop GPUs where RTP and RQ are always supported together, but on mobile many device manufacturers only support RQ. Interestingly on the device I tested it on, a Vivo X90 Pro, the hardware supports RTP but it's been disabled at the driver level - I only can assume the sample still works because that effort was botched.

Tested on:

General Checklist:

Please ensure the following points are checked:

Sample Checklist

If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist:

CLAassistant commented 9 months ago

CLA assistant check
All committers have signed the CLA.

SaschaWillems commented 9 months ago

Thanks for spotting this. You're 100% right, the ray tracing pipeline feature is not required when only using ray queries.

cmannett85-arm commented 9 months ago

The copyright checker is failing, but I've got this:

/* Copyright (c) 2021-2023, Holochip Corporation
 * Copyright (c) 2024, Arm Limited and Contributors

Am I supposed to update the other company's copyright dates too?

SaschaWillems commented 9 months ago

Yeah, that's sadly a limitation of the copyright checker. You also need to update the existing copyright.

@tomadamatkinson : Maybe that's something we can fix? I often have the same problem.

tomadamatkinson commented 9 months ago

@cmannett85-arm currently yes. We've had the same copyright check since the beginning.

I think technically we don't require copyright in a source file. As copyright is implied. We would need to decide as a WG on our copyright practices. Currently it is to bump all copyright owners if a file has changed.

There is a helper script here ./scripts/copyright.py main --fix which will automatically sort out the copyright for you.