gfx::Color is currently misused in many places. The DrawTargets expect
the color space to be in device space, e.g. what we are actually going
to draw using. Everything sitting above generally deals with sRGB, as
specified in CSS. Sometimes we missed the conversion from sRGB to device
space when issuing draw calls, and similarly sometimes we converted the
color to device space twice.
This patch splits the type in two. sRGBColor and DeviceColor now
represent sRGB and device color spaces respectively. DrawTarget only
accepts DeviceColor, and one can get a DeviceColor from an sRGBColor via
the ToDeviceColor helper API. The reftests now pass with color
management enabled for everything (e.g. CSS) instead of just tagged
raster images.
There will be a follow up patch to enable color management everywhere by
default on all supported platforms.
Differential Revision: https://phabricator.services.mozilla.com/D64771
--HG--
extra : moz-landing-system : lando
We include it everywhere because it's included from gfxTypes.h.
This should avoid including all the generated bindings _everywhere_.
Differential Revision: https://phabricator.services.mozilla.com/D62174
--HG--
extra : moz-landing-system : lando
This patch adds reporting the surface types used by the image frame in a
bit mask (such if it is a CAPTURE including a DATA_SHARED, the mask will
be 1 << CAPTURE | 1 << DATA_SHARED), as well as an estimated size
included in the report as decoded-unknown for when we do not know if the
surface is on the heap or the non-heap specifically. This is the default
implementation for a SourceSurface as well, so we should no longer have
the case where surfaces appear empty despite being in the cache. It also
makes requests being validated as always notable for reporting purposes.
Differential Revision: https://phabricator.services.mozilla.com/D61458
--HG--
extra : moz-landing-system : lando
On Windows, optimized surfaces can become invalid due to a device reset
or GPU process crash when D2D draw targets were being used. This is
because SourceSurfaceD2D1 checks if the D2D context has changed from the
one it was optimized for.
We assumed when creating a DrawableFrameRef that if we don't have
imgFrame::mRawSurface, then we must have an imgFrame::mOptSurface. This
is incorrect and causes SurfaceCache to believe it has a valid
DrawableSurface, even if the underlying optimized surface cannot be used
for drawing (or has already been freed).
If we can't draw the image, it goes missing from the screen. But since
the cache thinks it has a valid surface, we never actually redecode it.
With this patch, we now check if the imgFrame::mOptSurface has been
freed already, and if not, that it remains valid for drawing purposes.
If either condition is true, the SurfaceCache entry will be removed,
thus permitting a redecode attempt.
Differential Revision: https://phabricator.services.mozilla.com/D60834
--HG--
extra : moz-landing-system : lando
This dumps more information about the surface cache. Each surface has a
flag indicating whether or not it has finished decoding; incomplete
surfaces are now marked as such in the cache reports. We now also have
counters for various insertion errors, as well as the composition of the
cache (total vs locked images, total vs locked surfaces).
Differential Revision: https://phabricator.services.mozilla.com/D60450
--HG--
extra : moz-landing-system : lando
This was done by:
This was done by applying:
```
diff --git a/python/mozbuild/mozbuild/code-analysis/mach_commands.py b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
index 789affde7bbf..fe33c4c7d4d1 100644
--- a/python/mozbuild/mozbuild/code-analysis/mach_commands.py
+++ b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
@@ -2007,7 +2007,7 @@ class StaticAnalysis(MachCommandBase):
from subprocess import Popen, PIPE, check_output, CalledProcessError
diff_process = Popen(self._get_clang_format_diff_command(commit), stdout=PIPE)
- args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format]
+ args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format, '-sort-includes']
if not output_file:
args.append("-i")
```
Then running `./mach clang-format -c <commit-hash>`
Then undoing that patch.
Then running check_spidermonkey_style.py --fixup
Then running `./mach clang-format`
I had to fix four things:
* I needed to move <utility> back down in GuardObjects.h because I was hitting
obscure problems with our system include wrappers like this:
0:03.94 /usr/include/stdlib.h:550:14: error: exception specification in declaration does not match previous declaration
0:03.94 extern void *realloc (void *__ptr, size_t __size)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/malloc_decls.h:53:1: note: previous declaration is here
0:03.94 MALLOC_DECL(realloc, void*, void*, size_t)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozilla/mozalloc.h:22:32: note: expanded from macro 'MALLOC_DECL'
0:03.94 MOZ_MEMORY_API return_type name##_impl(__VA_ARGS__);
0:03.94 ^
0:03.94 <scratch space>:178:1: note: expanded from here
0:03.94 realloc_impl
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozmemory_wrap.h:142:41: note: expanded from macro 'realloc_impl'
0:03.94 #define realloc_impl mozmem_malloc_impl(realloc)
Which I really didn't feel like digging into.
* I had to restore the order of TrustOverrideUtils.h and related files in nss
because the .inc files depend on TrustOverrideUtils.h being included earlier.
* I had to add a missing include to RollingNumber.h
* Also had to partially restore include order in JsepSessionImpl.cpp to avoid
some -WError issues due to some static inline functions being defined in a
header but not used in the rest of the compilation unit.
Differential Revision: https://phabricator.services.mozilla.com/D60327
--HG--
extra : moz-landing-system : lando
rg -l 'mozilla/Move.h' | xargs sed -i 's/#include "mozilla\/Move.h"/#include <utility>/g'
Further manual fixups and cleanups to the include order incoming.
Differential Revision: https://phabricator.services.mozilla.com/D60323
--HG--
extra : moz-landing-system : lando
The inclusions were removed with the following very crude script and the
resulting breakage was fixed up by hand. The manual fixups did either
revert the changes done by the script, replace a generic header with a more
specific one or replace a header with a forward declaration.
find . -name "*.idl" | grep -v web-platform | grep -v third_party | while read path; do
interfaces=$(grep "^\(class\|interface\).*:.*" "$path" | cut -d' ' -f2)
if [ -n "$interfaces" ]; then
if [[ "$interfaces" == *$'\n'* ]]; then
regexp="\("
for i in $interfaces; do regexp="$regexp$i\|"; done
regexp="${regexp%%\\\|}\)"
else
regexp="$interfaces"
fi
interface=$(basename "$path")
rg -l "#include.*${interface%%.idl}.h" . | while read path2; do
hits=$(grep -v "#include.*${interface%%.idl}.h" "$path2" | grep -c "$regexp" )
if [ $hits -eq 0 ]; then
echo "Removing ${interface} from ${path2}"
grep -v "#include.*${interface%%.idl}.h" "$path2" > "$path2".tmp
mv -f "$path2".tmp "$path2"
fi
done
fi
done
Differential Revision: https://phabricator.services.mozilla.com/D55443
--HG--
extra : moz-landing-system : lando
Given the crash resolved in part 1, it is possible for the blob
rasterizer in the compositor process to still be using surfaces after
the animation has advanced to the next frame. With recycling this can be
problematic as the recycled surface will be reused for a future frame.
In an ideal world, the blob recording would use the animation's image
key instead, but the rasterizer doesn't have easy access to the mapping
table. As such, for any frames used in a blob recording, we now
explicitly mark them as non-recyclable and we will be forced to allocate
a new frame instead.
Differential Revision: https://phabricator.services.mozilla.com/D16192
Beyond the necessary reinitialization methods, we need to protect
ourselves from recycling a frame that some other entity in the browser
is still using. Generally speaking the animated surface will only be
used in imgFrame::Draw since we don't layerize animated images, which
will be safe. However with OMTP or blob recordings, we could retain a
reference to the surface outside the current stack context. Additional
if something calls RasterImage::GetImageContainer(AtSize) or
RasterImage::GetFrame(AtSize), it may also have a reference to the
surface for an indetermine period of time.
As such, if an imgFrame is a candidate for recycling, it will wrap
imgFrame::mLockedSurface in a RecyclingSourceSurface. Its job is to
track how many consumers there are still of the surface, so that after
we advance the animation, the decoder will know if there are still
outstanding consumers.
If the surface is still in use, it will block for a finite period of
time (the refresh interval) before giving up on reclaiming the surface,
and will allocate a new surface. The old surface can then remain in
circulation for as long as necessary without further blocking the
animation progression, since we stop recycling that surface/imgFrame.
Differential Revision: https://phabricator.services.mozilla.com/D7511
Since imgFrame::Draw will limit the drawing to only look at pixels that
we have written to and posted an invalidation for, there is no need to
hold the monitor while doing so. By taking the most expensive operation
outside the lock, we will minimize our chances of hitting contention
with the decoder thread.
A later part in this series will require that a surface be freed outside
the lock because it may end up reacquiring it. In addition to the
contention win, this change should facilitate that.
Differential Revision: https://phabricator.services.mozilla.com/D7510
At present, surface providers roll up all of their individual surfaces
into a single reporting unit. Specifically this means animated image
frames are all reported as a block. This patch removes that
consolidation and reports every frame as its own SurfaceMemoryReport.
This is important because each frame may have its own external image ID,
and we want to cross reference that with what we expect from the GPU
shared surfaces cache.
At present, surface providers roll up all of their individual surfaces
into a single reporting unit. Specifically this means animated image
frames are all reported as a block. This patch removes that
consolidation and reports every frame as its own SurfaceMemoryReport.
This is important because each frame may have its own external image ID,
and we want to cross reference that with what we expect from the GPU
shared surfaces cache.
DecoderFlags::BLEND_ANIMATION will cause the decoder to inject the
BlendAnimationFilter from the previous patch into the SurfacePipe filter
chain. All frames produced by this decoder will be complete, and
should be equivalent to the result outputted by FrameAnimator.
DecoderFlags::BLEND_ANIMATION will cause the decoder to inject the
BlendAnimationFilter from the previous patch into the SurfacePipe filter
chain. All frames produced by this decoder will be complete, and
should be equivalent to the result outputted by FrameAnimator.
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
We can easily use Maybe<DataSourceSurface::ScopedMap> instead of
allocated the map on the heap. This does require some minor changes to
ScopedMap to properly support moves, but should be much more efficient.
RawAccessFrameRef ensures there is a valid data pointer to the pixel
data for the frame. It is a common pattern for users of
RawAccessFrameRef to follow up with a request for the data pointer
shortly after creation. We can avoid an extra lock by exposing this data
pointer from RawAccessFrameRef, and populating it via
imgFrame::LockImageData.
We currently choose to set the animation parameters (blend method, blend
rect, disposal method, timeout) in imgFrame::Finish instead of
imgFrame::InitForDecoder. The decoders themselves already have access to
the necessary information at the time InitForDecoder is called, so there
is no reason to do this. Moving the configuration to initialization will
allow us to relax the mutex protection on these parameters.
This part simply reorganizes imgFrame, and subsequent parts will
introduce the necessary changes to SurfacePipe and decoders.
The shared memory handle reporting has been generalized to be an
external handle reporting. This is used for both shared memory, and for
volatile memory (on Android.) This will allow us to have a better sense
of just how many handles are being used by images on Android.
Additionally we were not properly reporting forced heap allocated
memory, if we were putting animated frames on the heap. This is because
we used SourceSurfaceAlignedRawData without implementing
AddSizeOfExcludingThis.
Andrew Osmond
Simon Giesecke
shindli
Emilio Cobos Álvarez
Gabriele Svelto
Csoregi Natalia
Nicolas Silva
Will Hawkins
Bas Schouten
Sylvestre Ledru
Narcis Beleuzu
Coroiu Cristina
Andi-Bogdan Postelnicu
Sebastian Hengst
Tristan Bourvon