[Webkit-unassigned] [Bug 72294] Add occludes() test to Region

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=72294





--- Comment #10 from Anders Carlsson <andersca at apple.com>  2011-11-14 16:16:58 PST ---
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > misclick..
> > > 
> > > "... to compute."
> > 
> > That could be easily fixed by making the segment and span vectors have an inline capacity - then it'd just be stack allocation:
> > 
> > 
> >         // FIXME: These vectors should have inline sizes. Figure out a good optimal value.
> >         Vector<int> m_segments;
> >         Vector<Span> m_spans;
> 
> From reading the code I don't see what you are suggesting. But please correct me where I am wrong.. If a Vector is given a size, then it will call VectorBufferBase::allocateBuffer() which calls fastMalloc() which calls malloc(). Where is there a stack allocation?

You can give Vector an inline size, like so:

Vector<int, 16> m_segments;

and unless the size of the vector is greater than 16 elements, everything will be allocated as part of the Vector (which is part of the Region).

> 
> Regarding point 1), I see nothing in Region::Shape::shapeOperation() that guarantees a single rect output for the intersection in the occludes() == true case. It simply adds segments as it goes along, there is nothing more global to consider pruning/combining segments/spans there. So, as I see it, the same test done in occludes() would need to be done on the result of the intersection() - the only change would be knowing that all segments intersect the query rect.

Region::Shape::canCoalesce has the logic for coalescing segments - but I think there's an even simpler way to do this.

Also, instead of occludes - let's call the function 'contains', because that's a more logical name. I think contains can be implemented like this:

bool Region::contains(const Region& region)
{
    Region result = intersect(*this, region);

    return result == region;
}

Which is much cleaner and reuses the already existing region operations. The region code is pretty complex as-is; let's not add more complexity to it if we can avoid it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.