- GetBitsPerPixel() and GetWidth() are no longer used.
- GetHeight() is now only used within nsBMPDecoder and can be renamed and
inlined into the header.
- GetImageData() can be inlined into the header.
The FileHeader and V5InfoHeader structs are shared by the BMP decoder and
encoder. But most of the fields within those structs are actually unused by the
decoder. It makes things clearer if we create a decoder-only struct that
contains the used fields, and then make FileHeader and V5InfoHeader only used
by the encoder. This patch does that.
This patch also renames BMPFileHeaders.h as BMPHeaders.h, which is now a better
name for it.
MakeUnique and its underlying |new| call will crash the program on
failure. This code was clearly written with fallible allocations in
mind, so let's make the allocations actually be fallible.
Currently we don't implement transparency at all in BMP images except for an
odd-duck case of BMPs within ICOs.
This patch does the following.
- It implements transparency properly for 16bpp and 32bpp images via bitfield
masking. (For 32bpp images this also requires handling colors via bitfield
masking.) The patch maintains the existing BMP-within-ICO transparency
handling.
- It also reworks BitFields::Value::Set().
* It now works correctly if the run of 1s goes all the way to bit 31 (the
old code didn't set mBitWidth).
* If the mask is 0, will give an mRightShift of 0 (old code gave 32, and
right-shifting by 32 is dodgy).
* It's now easier to read.
- It renames transparent.bmp as transparent-if-within-ico.bmp. Ironically
enough this file currently uses BITFIELDS compression and is WinBMPv5 format,
which means it contains well-specified alpha data. In order to use it to test
the hacky BMP-within-ICO transparency scheme the patch changes it to be
WinBMPv3 format with RGB compression (i.e. no compression). I left the
now-excess bytes (including the bitfields) in the info header in place
because that's allowed -- thanks to the start of the pixel data being
specified by the |dataoffset| field -- they'll just be ignored.
- It tweaks the naming of the relevant gtests and some of their finer details
to work with the new way of doing things.
This fixes all four remaining failures in bmpsuite.
Currently we don't read BMP bitfields during metadata decoding. But we'll need
to in order implement alpha, because we need to know during metadata decoding
if alpha is used.
This patch moves code around to achieve this (and adds the required
mMayHaveTransparency field). The change has no noticeable effect for now.
The wrinkle here is that while we can pass a UniquePtr to
ConvertHostARGBRow, we can't pass UniquePtr to
EncodeImageDataRow{24,32}, as the latter get called with raw pointers
out of our control.
These functions are only used in nsBMPDecoder.cpp; we don't need them
anywhere else. The only other place they might get used would be the
BMP encoder, but the encoder appears to have its own routines for
setting pixel data, which don't overlap very well with the decoder's.
mTouchedTime is not appropriate for this as it is updated when an image
load re-uses the same imgRequest, especially as it has one second
granularity. A timestamp that is updated every time the backing file is
re-read should work better.
A millisecond granularity timestamp would be preferable, and would be
achievable on most or all supported platforms. But some older
filesystems have timestamp granularity of a second or worse, notably
ext3 and FAT32 (and even ext4 filesytems created with inode_size < 256
bytes, e.g. with 'mke2fs -t small' - see
https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout#Inode_Timestamps
for details.)
The bulk of this commit was generated with a script, executed at the top
level of a typical source code checkout. The only non-machine-generated
part was modifying MFBT's moz.build to reflect the new naming.
CLOSED TREE makes big refactorings like this a piece of cake.
# The main substitution.
find . -name '*.cpp' -o -name '*.cc' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
xargs perl -p -i -e '
s/nsRefPtr\.h/RefPtr\.h/g; # handle includes
s/nsRefPtr ?</RefPtr</g; # handle declarations and variables
'
# Handle a special friend declaration in gfx/layers/AtomicRefCountedWithFinalize.h.
perl -p -i -e 's/::nsRefPtr;/::RefPtr;/' gfx/layers/AtomicRefCountedWithFinalize.h
# Handle nsRefPtr.h itself, a couple places that define constructors
# from nsRefPtr, and code generators specially. We do this here, rather
# than indiscriminantly s/nsRefPtr/RefPtr/, because that would rename
# things like nsRefPtrHashtable.
perl -p -i -e 's/nsRefPtr/RefPtr/g' \
mfbt/nsRefPtr.h \
xpcom/glue/nsCOMPtr.h \
xpcom/base/OwningNonNull.h \
ipc/ipdl/ipdl/lower.py \
ipc/ipdl/ipdl/builtin.py \
dom/bindings/Codegen.py \
python/lldbutils/lldbutils/utils.py
# In our indiscriminate substitution above, we renamed
# nsRefPtrGetterAddRefs, the class behind getter_AddRefs. Fix that up.
find . -name '*.cpp' -o -name '*.h' -o -name '*.idl' | \
xargs perl -p -i -e 's/nsRefPtrGetterAddRefs/RefPtrGetterAddRefs/g'
if [ -d .git ]; then
git mv mfbt/nsRefPtr.h mfbt/RefPtr.h
else
hg mv mfbt/nsRefPtr.h mfbt/RefPtr.h
fi
This commit was generated using the following script, executed at the
top level of a typical source code checkout.
# Don't modify select files in mfbt/ because it's not worth trying to
# tease out the dependencies currently.
#
# Don't modify anything in media/gmp-clearkey/0.1/ because those files
# use their own RefPtr, defined in their own RefCounted.h.
find . -name '*.cpp' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
grep -v 'mfbt/RefPtr.h' | \
grep -v 'mfbt/nsRefPtr.h' | \
grep -v 'mfbt/RefCounted.h' | \
grep -v 'media/gmp-clearkey/0.1/' | \
xargs perl -p -i -e '
s/mozilla::RefPtr/nsRefPtr/g; # handle declarations in headers
s/\bRefPtr</nsRefPtr</g; # handle local variables in functions
s#mozilla/RefPtr.h#mozilla/nsRefPtr.h#; # handle #includes
s#mfbt/RefPtr.h#mfbt/nsRefPtr.h#; # handle strange #includes
'
# |using mozilla::RefPtr;| is OK; |using nsRefPtr;| is invalid syntax.
find . -name '*.cpp' -o -name '*.mm' | xargs sed -i -e '/using nsRefPtr/d'
# RefPtr.h used |byRef| for dealing with COM-style outparams.
# nsRefPtr.h uses |getter_AddRefs|.
# Fixup that mismatch.
find . -name '*.cpp' -o -name '*.h'| \
xargs perl -p -i -e 's/byRef/getter_AddRefs/g'
This patch implements proper color-scaling, instead of bit-shifting, and uses
it for 16bpp images.
It also cleans up the code relating to color masking in the process, by making
BitFields a proper class and introducing the Value class within it.
This fixes sub-optimal handling of four images in bmpsuite.
Two-space indents is the Gecko standard, and it's what nsBMPDecoder.cpp uses.
This patch changes nsBMPEncoder.h to use two-space indents as well. This will
help avoid possible mis-indentation if code is moved from the .h to the .cpp or
vice versa (as subsequent patches in this bug will do).
Also, it changes some of the // comments on public methods to doxygen-style ///
comments.
We have 52 passes and 5 known fails. Three of the passes have higher fuzziness
allowances than they should, so really there are 8 files that we need to
improve on.
This patch is a major overhaul of nsBMPDecoder.
The patch improves the code in the following ways.
- It converts nsBMPDecoder to use StreamingLexer, which makes it much easier to
read.
- It adds a detailed comment about the BMP format at the top of
nsBMPDecoder.cpp.
- It fixes lots of inconsistent indenting.
- It moves |bihsize| from |mBFH| to |mBIH| to match the file format and common
sense. The avoids the need for the confusing LENGTH/INTERNAL_LENGTH
distinction.
- It renames most of the types in BMPFileHeader.h, so they have better names,
in StudlyCaps form, and within the new |bmp| namespace.
- It removes the BMP_HEADER_LENGTH struct and inlines its values directly into
the two places they were used.
- It removes the MOZ_LOG logging done on some of the failure cases. (Most
failure cases lacked logging so why bother with some?)
- It removes over 200 lines of code, despite the addition of the big format
comment.
The patch changes the way BMPs are decoded as follows.
- It adds stricter testing of the InfoHeader length, rejecting files with bad
values.
- It moves all header sanity checking that can lead to file rejection into the
metadata decode phase. (Previously, bpp/compression consistency checking did
not occur during a metadata decode.)
- It removes BMPINFOHEADER::ALPHABITFIELDS, which was (a) a weird WinCE-only
thing, and (b) we didn't actually allow it, and (c) we used the value 4
instead of 6(!).
- It rejects the previously-accepted compression==RLE4 && bpp=1 combination
because it doesn't make sense.
- It removes a fudge in RLE absolute mode handling that permitted one pixel too
many in a row but only if the row's width was odd(!)
- It now rejects a file with a negative gap between the color table and the
pixel data.
The patch leaves the following problems unaddressed.
- If bpp==32 we totally ignore compression==BITFIELDS and treat it like
compression=RGB.
- Transparency as specified in WinBMPv{4,5} isn't handled at all.
These will be fixed in follow-ups.
All these changes affect (for the better) the results of the following tests
that will be added in part 2:
- g/pal8v4.bmp
- g/pal8v5.bmp
- q/pal8os2sp.bmp
- q/pal8os2v2.bmp
- q/pal8os2v2-16.bmp
- b/badheadersize.bmp
- b/badpalettesize.bmp
- b/badrle.bmp
|getReportsForThisProcess| differs from |getReports| in that it is limited to current process and is synchronous. When asynchronous memory reporters are added the function will no longer be able tobe synchronous. There isn't much utility in only measuring the current process, so we can remove the function and switch existing users to |getReports|.
The conversion is as follows:
- GraphicsFilter::FILTER_NEAREST == gfx::Filter::POINT
- GraphicsFilter::FILTER_GOOD == gfx::Filter::GOOD
- GraphicsFilter::FILTER_BEST == gfx::Filter::LINEAR
Also typedef GraphicsFilter to gfx::Filter; this will be removed in the next
patch.
These changes mean ToFilter() and ThebesFilter() are no longer needed.
|getReportsForThisProcess| differs from |getReports| in that it is limited to current process and is synchronous. When asynchronous memory reporters are added the function will no longer be able tobe synchronous. There isn't much utility in only measuring the current process, so we can remove the function and switch existing users to |getReports|.
The orientation matrix converts from decoded image space to oriented image space. The invalidation rect is in decoded image space. So we need to use the orientation matrix to convert it to oriented image space, not it's inverse.
