1429792 - Expose const pointer to BaseRect internal data for special and limited use

Status: NEW

(Core :: Graphics, enhancement, P3)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

Example is a Hash() method https://dxr.mozilla.org/mozilla-central/source/layout/painting/MaskLayerImageCache.h#112 that wants a pointer to data in order to hash, but doesn't try to interpret the data in any particular way.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Bug 1429792: Allow const pointer access to underlying BaseRect data, for special and limited use. .schouten

Review commit: https://reviewboard.mozilla.org/r/212072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212072/

Comment 2

[Daniel Holbert [:dholbert]]

This didn't occur to me yesterday, but now I'm wondering -- can't we just directly use the Rect pointer itself for hashing etc, rather than needing a pointer to its member data?

e.g. in MaskLayerImageCache.h's hash function, couldn't we replace this...
      PLDHashNumber hash = HashBytes(&mRect.x, 4*sizeof(gfxFloat));
...with this:
      HashBytes(&mRect, sizeof(mRect));
?


I can imagine this general approach being problematic if we expected arbitrarily-filled packing space between the member-variables or stuff like that, but I don't think that should happen here...

Comment 3

[Daniel Holbert [:dholbert]]

(Sorry, I was of course missing a  "PLDHashNumber hash =" in my possible-replacement code -- but you get the idea.)

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

I don't have a preference.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 8941844 [details]
Bug 1429792: Allow const pointer access to underlying BaseRect data, for special and limited use. .schouten

https://reviewboard.mozilla.org/r/212072/#review217980

If that's the only usecase, for the sake of simplicity I agree with Daniel.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

So far, the only use case.  I'm going to keep this open with bug 1390110 in mind.