On Thu, 16 Sept 2021 at 20:34, Stefan Dösinger stefandoesinger@gmail.com wrote:
Would it make sense to do a binary search here instead of trying every value? (Similar to what we do in wined3d_adapter_find_polyoffset_scale())
Between 1/64 and 1/1024 it might. I don't want to search the space between 1/1024 and 0 for the reason I explained in some comment (unpredictability wrt to other viewport dimensions). For the 5 non-zero values we're testing I don't know if it makes any difference in practise. Yeah I know cache locality isn't a concern here because we're not searching memory contents :-) .
Though for clarity: Do you suggest searching non-power-of-two values too? I am not sure if that will give us anything of use. I've avoided them because I expect powers of two to be more reliable re rounding when we add them to actual geometry positions.
Well, there are different variant we could do. The most straightforward one is perhaps to simply do a binary search of the values in test_array[], it's already sorted. Alternatively, we'd set e.g. "low = 6, high = 10", and then "value = -1.0f / (1u << cur)". Perhaps there are also just few enough values that it's not worth it. I'd stick to powers of two, yes.
Do we really need a FIXME for that? Getting e.g. a bottom-right result would perhaps be unexpected, but it doesn't seem particularly concerning.
A *-right result is the most obvious case, but I also saw the MacOS driver completely discard the test draw at certain magic offsets (1^-24.5 or something along those lines if I remember right); The top edge wasn't yet on the pixel but the bottom edge was already moved away.
Bottom line, we'd potentially apply a nudge for the wrong reason. The nudge can break things just as much as fix things. Seems worthy of a FIXME to me.
I am persuadable to returning TRUE in case of an unexpected result. That way we'd either not apply a quirk if things go wrong or just apply too small a nudge (if we keep the linear search). In that case I am less invested in the FIXME, but we'd depart more from the current behaviour.
We could certainly get broken results here, yes, and those would be worth a FIXME or even a winediag message. I think validation rules are just the following though:
- Exactly one of the centre four values in readback[] is 0xffff0000. - All other values in readback[] are 0xff0000ff.
Provided we have a valid result, we can then just "return readback[3][3] == 0xffff0000;"