Re: [webkit-dev] Improve selection with floats

[Javier Fernandez]

I'd really appreciate any feedback on this thread from layout experts. I
have a few ideas and some time to work on them, but I'd need to know if
WebKit is interested on exploring them. I know it's a complex logic and
the lack of specification does not help at all.

Thanks.


On 07/09/17 16:40, Javier Fernandez wrote:
> Hi,
>
> I've been working lately on some cases where selection shows an
> unpredictable behavior when applied to cases with float elements;
> basically, selection boundaries jump when dragging over floats. I've
> found this old bug (https://webkit.org/b/101771) which described some of
> these cases. I've attached another case with the same problem.
>
> This weird Selection's behavior is also present in Blink and Gecko:
>   - https://crbug.com/758526
>   - https://bugzilla.mozilla.org/show_bug.cgi?id=1397514
>
> I'm aware of the issues we have with Selection when the Render Tree
> differs from the DOM Tree's structure, but that's not the case of the
> examples I've been evaluating so far; I think for these cases we can do
> better.
>
> My theory is that the root cause of this issue in most, if not all, the
> cases is that we are not considering floats as valid HitTest candidates.
> Additionally, we exclude floats from the positionForPoint logic,
> implemented in the different render objects.
>
> I'm evaluating a preliminary approach based on considering floats as
> valid HitTest candidates. I started to address a very specific case in
> https://webkit.org/b/176096 and, if it gets enough support, I have plans
> to continue addressing as many cases as possible.
>
> I think we have to assume that we will have issues with floats when they
> create complex render trees, like it happens with other layout models,
> but at least we can improve the simplest cases.
>
> I appreciate any feedback on my current approach and reviews of my
> patches, if we finally want to give this proposal a shot.
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Forward.h's Vector

[JF Bastien]

> On Sep 13, 2017, at 11:07, Maciej Stachowiak  wrote:
> 
> 
> 
>> On Sep 13, 2017, at 8:11 AM, JF Bastien > > wrote:
>> 
>> Hello WebCritters,
>> 
>> I’m moving some code around, and one particular header I have is included 
>> everywhere in JSC so I’d like it to be lightweight and include as few other 
>> headers as possible. It unfortunately uses WTF::Vector, but it only does so 
>> as a pointer:
>> 
>> void ohNoes(Vector*);
>> 
>> It seems like something a forward declaration would fix nicely, and oh joy 
>> and wonder we have Forward.h just for this! Here’s what it does:
>> 
>> template> minCapacity, typename Malloc> class Vector;
>> 
>> That’s nice and great for T, but all the other template parameters are SOL 
>> because Vector is actually declared with default template parameters:
>> 
>> template> CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc>
>> class Vector : private VectorBuffer {
>> 
>> The extra painful thing is that, contrary to what I originally thought, one 
>> cannot declare Vector in Forward.h and then define it in Vector.h with the 
>> same defaults twice! Ideally the compiler would just yell at a mismatch, but 
>> instead it says error: template parameter redefines default argument (thanks 
>> clang, that’s exactly what I’m trying to do, just tell me if you catch a 
>> mismatch and ODR away otherwise).
>> 
>> Here’s what I propose:
>> 
>> Change the forward declaration in Forward.h to contain the default template 
>> parameters (and forward-declare CrashOnOverflow).
>> Remove these default template parameters from Vector.h.
>> Include Forward.h in Vector.h.
>> Optionally, if the WebCritters think it useful, leave a comment just above 
>> the Vector definition redirecting to Forward.h for defaults (or more fancy, 
>> use size_t inlineCapacity /*=0*/ style like LLVM’s codebase does, and have a 
>> tool that checks for consistency).
>> Optionally, try to fix C++20 so this isn’t A Thing anymore. Note that my 
>> hopes are low because of modules (why fix it if modules will magically make 
>> this not a thing).
>> 
>> Alternatively, I could just include Vector.h and be done with it.
>> 
>> Thoughts?
> 
> Is there anything in Forward.h that should not be included everywhere you 
> include Vector.h, whether for efficiency or any other reasons? That's the 
> only potential thing I can think of that may be wrong with this plan. In that 
> case, the simple fix would be to have a separate VectorForward.h which can be 
> included by both.

I thought about that, and it seems like Forward.h is so tiny that it shouldn’t 
be an issue. If it ever starts including other WTF things then we should 
definitely do what you say, but I don’t think we should at the moment. Or put 
another way, if Forward.h doesn’t purely forward-declare things then it’s doing 
it wrong. :-)

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Forward.h's Vector

[Maciej Stachowiak]

> On Sep 13, 2017, at 8:11 AM, JF Bastien  wrote:
> 
> Hello WebCritters,
> 
> I’m moving some code around, and one particular header I have is included 
> everywhere in JSC so I’d like it to be lightweight and include as few other 
> headers as possible. It unfortunately uses WTF::Vector, but it only does so 
> as a pointer:
> 
> void ohNoes(Vector*);
> 
> It seems like something a forward declaration would fix nicely, and oh joy 
> and wonder we have Forward.h just for this! Here’s what it does:
> 
> template minCapacity, typename Malloc> class Vector;
> 
> That’s nice and great for T, but all the other template parameters are SOL 
> because Vector is actually declared with default template parameters:
> 
> template CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc>
> class Vector : private VectorBuffer {
> 
> The extra painful thing is that, contrary to what I originally thought, one 
> cannot declare Vector in Forward.h and then define it in Vector.h with the 
> same defaults twice! Ideally the compiler would just yell at a mismatch, but 
> instead it says error: template parameter redefines default argument (thanks 
> clang, that’s exactly what I’m trying to do, just tell me if you catch a 
> mismatch and ODR away otherwise).
> 
> Here’s what I propose:
> 
> Change the forward declaration in Forward.h to contain the default template 
> parameters (and forward-declare CrashOnOverflow).
> Remove these default template parameters from Vector.h.
> Include Forward.h in Vector.h.
> Optionally, if the WebCritters think it useful, leave a comment just above 
> the Vector definition redirecting to Forward.h for defaults (or more fancy, 
> use size_t inlineCapacity /*=0*/ style like LLVM’s codebase does, and have a 
> tool that checks for consistency).
> Optionally, try to fix C++20 so this isn’t A Thing anymore. Note that my 
> hopes are low because of modules (why fix it if modules will magically make 
> this not a thing).
> 
> Alternatively, I could just include Vector.h and be done with it.
> 
> Thoughts?

Is there anything in Forward.h that should not be included everywhere you 
include Vector.h, whether for efficiency or any other reasons? That's the only 
potential thing I can think of that may be wrong with this plan. In that case, 
the simple fix would be to have a separate VectorForward.h which can be 
included by both.

 - Maciej



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Forward.h's Vector

[Myles C. Maxfield]

> On Sep 13, 2017, at 8:11 AM, JF Bastien  wrote:
> 
> Hello WebCritters,
> 
> I’m moving some code around, and one particular header I have is included 
> everywhere in JSC so I’d like it to be lightweight and include as few other 
> headers as possible. It unfortunately uses WTF::Vector, but it only does so 
> as a pointer:
> 
> void ohNoes(Vector*);
> 
> It seems like something a forward declaration would fix nicely, and oh joy 
> and wonder we have Forward.h just for this! Here’s what it does:
> 
> template minCapacity, typename Malloc> class Vector;
> 
> That’s nice and great for T, but all the other template parameters are SOL 
> because Vector is actually declared with default template parameters:
> 
> template CrashOnOverflow, size_t minCapacity = 16, typename Malloc = FastMalloc>
> class Vector : private VectorBuffer {
> 
> The extra painful thing is that, contrary to what I originally thought, one 
> cannot declare Vector in Forward.h and then define it in Vector.h with the 
> same defaults twice! Ideally the compiler would just yell at a mismatch, but 
> instead it says error: template parameter redefines default argument (thanks 
> clang, that’s exactly what I’m trying to do, just tell me if you catch a 
> mismatch and ODR away otherwise).
> 
> Here’s what I propose:
> 
> Change the forward declaration in Forward.h to contain the default template 
> parameters (and forward-declare CrashOnOverflow).
> Remove these default template parameters from Vector.h.
When looking for default parameters, I would expect to find them next to the 
Vector definition. If we do this, #4 should be required.
> Include Forward.h in Vector.h.
> Optionally, if the WebCritters think it useful, leave a comment just above 
> the Vector definition redirecting to Forward.h for defaults (or more fancy, 
> use size_t inlineCapacity /*=0*/ style like LLVM’s codebase does, and have a 
> tool that checks for consistency).
> Optionally, try to fix C++20 so this isn’t A Thing anymore. Note that my 
> hopes are low because of modules (why fix it if modules will magically make 
> this not a thing).
> 
> Alternatively, I could just include Vector.h and be done with it.
> 
> Thoughts?
> 
> JF
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Forward.h's Vector

[JF Bastien]

Hello WebCritters,

I’m moving some code around, and one particular header I have is included 
everywhere in JSC so I’d like it to be lightweight and include as few other 
headers as possible. It unfortunately uses WTF::Vector, but it only does so as 
a pointer:

void ohNoes(Vector*);

It seems like something a forward declaration would fix nicely, and oh joy and 
wonder we have Forward.h just for this! Here’s what it does:

template class Vector;

That’s nice and great for T, but all the other template parameters are SOL 
because Vector is actually declared with default template parameters:

template
class Vector : private VectorBuffer {

The extra painful thing is that, contrary to what I originally thought, one 
cannot declare Vector in Forward.h and then define it in Vector.h with the same 
defaults twice! Ideally the compiler would just yell at a mismatch, but instead 
it says error: template parameter redefines default argument (thanks clang, 
that’s exactly what I’m trying to do, just tell me if you catch a mismatch and 
ODR away otherwise).

Here’s what I propose:

Change the forward declaration in Forward.h to contain the default template 
parameters (and forward-declare CrashOnOverflow).
Remove these default template parameters from Vector.h.
Include Forward.h in Vector.h.
Optionally, if the WebCritters think it useful, leave a comment just above the 
Vector definition redirecting to Forward.h for defaults (or more fancy, use 
size_t inlineCapacity /*=0*/ style like LLVM’s codebase does, and have a tool 
that checks for consistency).
Optionally, try to fix C++20 so this isn’t A Thing anymore. Note that my hopes 
are low because of modules (why fix it if modules will magically make this not 
a thing).

Alternatively, I could just include Vector.h and be done with it.

Thoughts?

JF

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev