On Tue May 20 15:01:18 2025 +0000, Jinoh Kang wrote:
`RDW_FRAME` means nothing when `RDW_INVALIDATE` is not set[^1], overloading it is arguably a flag abuse. Meanwhile, there is nothing weird about `frame == 1 && flags & RDW_NOFRAME` since `frame` is just an internal variable and does not have such meaning attached. (That would also break the `RDW_VALIDATE | RDW_FRAME | RDW_NOFRAME`-with-region case, since `RDW_FRAME` is no longer being ignored which results in more flickers in NC area. But that's a weird flag combination and I guess we're not testing for it anyway, so could be a bit theoretical case as long as no apps actually do that.) If this patch is a must for this serie then I don't have strong objection, but this nevertheless comes as a maintainability/inconsistency issue since now the reader has to keep track of what `RDW_VALIDATE | RDW_FRAME` means in which context (externally it means nothing, internally it means "validating child window including its frame"). [^1]: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-redra...
The patch isn't necessary but I was aiming at reducing the number of parameters overall in a MR that added two more, and I though it would be better to remove one seemingly independent parameter that actually isn't.
I don't think having to track down 6 different calls sites to figure under which condition that additional parameter can be set to a 1/0 constant, based on seemingly arbitrary decision as it is hardcoded in most of them, is easier to maintain than taking the information where it actually comes from (the RDW_FRAME flag), with yes, a local abuse of flag to carry a local information through a recursive call.