Bug 238063 - [WebGPU] maxAnisotropy > 16 is clamped, rather than illegal
Summary: [WebGPU] maxAnisotropy > 16 is clamped, rather than illegal
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-17 19:52 PDT by Myles C. Maxfield
Modified: 2022-03-22 07:55 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.45 KB, patch)
2022-03-17 19:54 PDT, Myles C. Maxfield
kkinnunen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2022-03-17 19:52:27 PDT
[WebGPU] maxAnisotropy > 16 is clamped, rather than illegal
Comment 1 Myles C. Maxfield 2022-03-17 19:54:06 PDT
Created attachment 455062 [details]
Patch
Comment 2 Kimmo Kinnunen 2022-03-21 01:03:33 PDT
Comment on attachment 455062 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455062&action=review

> Source/WebGPU/WebGPU/Sampler.mm:195
> +    samplerDescriptor.maxAnisotropy = std::min(descriptor.maxAnisotropy, static_cast<uint16_t>(16));

sometimes less tokens for the human to parse, less ambiguous(not sure it compiles without errors for us, maybe?)
  std::min<uint16_t>(descriptor.maxAnisotropy, 16);
Comment 3 Myles C. Maxfield 2022-03-21 18:13:39 PDT
Comment on attachment 455062 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455062&action=review

> Source/WebGPU/ChangeLog:12
> +

Covered by api/operation/sampling/anisotropy.spec.ts
Comment 4 Myles C. Maxfield 2022-03-21 18:19:12 PDT
Committed r291593 (248687@trunk): <https://commits.webkit.org/248687@trunk>
Comment 5 Radar WebKit Bug Importer 2022-03-21 18:20:17 PDT
<rdar://problem/90605401>
Comment 6 Darin Adler 2022-03-22 07:55:18 PDT
Comment on attachment 455062 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455062&action=review

>> Source/WebGPU/WebGPU/Sampler.mm:195
>> +    samplerDescriptor.maxAnisotropy = std::min(descriptor.maxAnisotropy, static_cast<uint16_t>(16));
> 
> sometimes less tokens for the human to parse, less ambiguous(not sure it compiles without errors for us, maybe?)
>   std::min<uint16_t>(descriptor.maxAnisotropy, 16);

I don’t absolutely love this idiom, because when I read std::min<uint16_t>(descriptor.maxAnisotropy, 16) I think "is maxAnisotropy bigger than 16-bit, because if it is, this thing will chop the high bits". Because of that I would write the less terse:

    constexpr uint16_t maxMaxAnistropy = 16;
    samplerDescriptor.maxAnisotropy = std::min(descriptor.maxAnisotropy, maxMaxAnistropy);