Currently fMSAAResolvesAutomatically would be enabled when the
system supports 'GL_EXT_multisampled_render_to_texture'. While
for d3d11 backend, this path with automatically resolving still
has some bugs[1] need to be fixed. We could make this path fall
back to the default path before that still uses two different
fbos. By doing this, we could guarantee this dmsaa path is usable
for d3d11 backend in the functionality. Besides, some graphics
workloads could be benefited by the dmsaa fast path.
[1] https://bugs.chromium.org/p/angleproject/issues/detail?id=6030
Change-Id: I949024e7c3546f05af4eea45ef7959e9f1078b52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545616
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
Our implementation currently requires GrGLCaps::fTransferBufferType to
be set to something other than kNone for this to work. We haven't
enabled transfer surface/buffer transfers yet and so are not
setting this value. To do so we'd have to more thoroughly understand
the additional buffer restrictions in WebGL compared to GLES and it
isn't currently a priority.
Bug: skia:13461
Change-Id: I033b9d2c7a3116bdc95bcdbae1e4ac86c9f1b2c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551894
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
* Changed one static_assert to SkASSERT in RangeSelector.cpp:42
Change-Id: I12815a8817816261bb30f5412432109ed46826fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551892
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Currently, the SkRemoteGlyphCache can't convert an SkTextBlob
to a Slug because it holds minimal glyph information.
For GlyphVector the remote cache has enough information to
serialize the GlyphVector, but not enough to draw it. In this
first step allow GlyphVector to work with either an SkStrike or
a RemoteStrike.
Change-Id: I8c261d84b40a6a69ac0077aaaeed9d318ae72c23
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551890
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Label temprory offscreen textures for draws. In this CL, we will
label texture for gradient from GrGradientShader which will help
labeling parts of SkImages too.
Bug: chromium:1164111
Change-Id: Iea49598f7632bb2edfaef21a0956771af5833cc1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550736
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
The big change here is having the C++ toolchain use
Bazel platforms instead of the C++ specific flags/setup.
In Bazel, platforms are a general purpose way to define
things like os, cpu architecture, etc. We were not using
platforms previously, because the best documentation at
the time focused on the old ways.
However, the old ways were clumsy/difficult when trying
to manage cross-compilation, specifically when trying
to have a Mac host trigger a build on our Linux RBE
system targeting a Linux x64 system. Thus, rather than
keep investing in the legacy system, this CL migrates
us to using platforms where possible.
Suggested background reading to better understand this CL:
- https://bazel.build/concepts/platforms-intro
- https://bazel.build/docs/platforms
- https://bazel.build/docs/toolchains#registering-building-toolchains
The hermetic toolchain itself is not changing in this CL
(and likely does not need to), only how we tell Bazel
about it (i.e. registering it) and how Bazel decides
to use it (i.e. resolving toolchains).
Here is my understanding of how platforms and toolchains
interact (supported by some evidence from [1][2])
- Bazel needs to resolve platforms for the Host, Execution,
and Target.
- If not specified via flags, these are the machine from
which Bazel is invoked, aka "@local_config_platform//:host".
- With this CL, the Host could be a Mac laptop, the Execution
platform is our Linux RBE pool, and the Target is "a Linux
system with a x64 CPU"
- To specify the Host, that is, describe to Bazel the
capabilities of the system it is running on, one can
set --host_platform [3] with a label pointing to a platform()
containing the appropriate settings. Tip: have this
platform inherit from @local_config_platform//:host
so it can add to any of the constraint_settings and
constraint_values that Bazel deduces automatically.
- To specify the Target platform(s), that is, the system
on which a final output resides and can execute, one
can set the --platforms flag with a label referencing
a platform().
- Bazel will then choose an execution platform to fulfill
that request. Bazel will look through a list of available
platforms, which can be augmented* with the
--extra_execution_platforms. Platforms specified by this
flag will be considered higher than the default platforms!
- Having selected the appropriate platforms, Bazel now
needs to select a toolchain to actually run the actions
of the appropriate type.
- Bazel looks through the list of available toolchains
and finds one that "matches" the Execution and the Target
platform. This means, the toolchain's exec_compatible_with
is a strict subset of the Execution platform and
the toolchain's target_compatible_with is a strict subset
of the Target platform. To register toolchains* (i.e. add
them to the resolution list), we use --extra_toolchains.
Once Bazel finds a match, it stops looking.
Using --toolchain_resolution_debug=".*" makes Bazel log
how it is resolving these toolchains and what execution
platform it picked.
* We can also register execution platforms and toolchains in
WORKSPACE.bazel [4], but the flags come with higher priority
and that made resolution a bit tricky. Also, when we want
to conditionally add them (e.g. --config=linux_rbe), we
cannot remove them conditionally in the WORKSPACE.bazel file.
The above resolution flow directly necessitated the changes
in this CL.
Example usage of the new configs and platforms:
# Can be run on a x64 Linux host and uses the hermetic toolchain.
bazel build //:skia_public
# Can be run on Mac or Linux and uses the Linux RBE system along
# with the hermetic toolchain to compile a binary for Linux x64.
bazel build //:skia_public --config=linux_rbe --config=for_linux_x64
# Shorthand for above
bazel build //:skia_public --config=for_linux_x64_with_rbe
Notice we don't have to type out --config=clang_linux anymore!
That was due to me reading the Bazel docs more carefully and
realizing we can set options for *all* Bazel build commands.
Current Limitations:
- Targets which require a py_binary (e.g. Dawn's genrules)
will not work on RBE when cross compiling because the
python runtime we download is for the host machine, not
the executor. This means //example:hello_world_dawn does
not work on Mac when cross-compiling via linux_rbe.
- Mac M1 linking not quite working with SkOpts settings.
Probably need to set -target [5]
Suggested Review order:
- toolchain/BUILD.bazel Notice how we do away with
cc_toolchain_suite for toolchain. These have the same
role: giving Bazel the information about where a toolchain
can run. The platforms one is more expressive (IMO), allowing
us to say both where to run the toolchain and what it can
make. In order to more easily force the use of our hermetic
toolchain, but also allow the hermetic toolchain to be used
on RBE, we specify "use_hermetic_toolchain" only on the target,
because the RBE image does not have the hermetic toolchain
on it by default (but can certainly run it).
- bazel/platform/BUILD.bazel to see the custom constraint_setting
and corresponding constraint_value. The names for both of these
are completely arbitrary - they do not need to have any deeper
meaning or relation to any file or Docker image or system or
any other constraints. Think of the constraint_setting as
an Enum and the constraint_value being the one and only member.
We need to pass around a constant value, not a type, so we
need to provide the constraint_value (e.g. in toolchain/BUILD.bazel)
but not a constraint_setting. However we need a
constraint_setting declared so we can make a constraint_value
of that "type".
Notice the platform declared here - it allows us to force
Bazel to use the hermetic toolchain because of the extra
constraint_value.
- .bazelrc I set a few flags that will be on for all
bazel build commands. Importantly, this causes the C++
build logic to use platforms and not the old, bespoke way.
I also found a way to avoid using the local toolchain on
the host, which will hopefully lead to clearer errors
if platforms are mis-specified instead of odd compile
errors because the host toolchain is too old or something.
There are also a few RBE settings tweaked to be a bit
more modern, as well the new shorthands for specifying
target platforms (e.g. for_linux_x64).
- bazel/buildrc where we have to turn off the platforms
logic for emscripten https://github.com/emscripten-core/emsdk/issues/984
- bazel/rbe/BUILD.bazel for a fix in the platform description
that makes it work on Mac.
- Notice that _m1 has been removed from the mac-related toolchain
files because the same toolchain should work on both
architectures.
- All other changes in any order.
[1] https://bazel.build/docs/toolchains#debugging-toolchains
[2] https://bazel.build/docs/toolchains#toolchain-resolution
[3] https://bazel.build/reference/command-line-reference
[4] https://bazel.build/docs/toolchains#registering-building-toolchains
[5] 17dc3f16fc/gn/skia/BUILD.gn (L258-L271)
Change-Id: I515c114099d659639a808f74e47d489a68b7af62
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549737
Reviewed-by: Erik Rose <erikrose@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
The anisomips GM appears to snap an image from a surface and samples it.
Because we haven't implemented that yet, validation of that texture
fails, which in turns means we don't insert a Recording into the
Context, and hence that Context has no CommandBuffer.
Added a message when we fail to validate TextureInfo, and pushed the
CommandBuffer submit in GraphiteMetalWindowContext inside the block
that checks to see if we have a Recording.
Change-Id: I74d8886a56ff703f7550bde321a4c3efbac66f30
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550708
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Refactor the implementation to decouple frame generators
(FrameGenerator: cpu, gpu, picture) from frame consumers (FrameSink:
png, skp, mp4, null).
Add a GPU frame generator using async readbacks. Unlike other
generators, which execute on a thread pool, selecting the GPU generator
forces single-thread execution.
Also add a couple of backend-specific build targets (skottie_tool_cpu,
skottie_tool_gpu) to facilitate binary size experiments.
Change-Id: Id59e230b3861afe5bf9b7ecfc710d672f38eeaaf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551237
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
If we are going to create a new snippet ID every time a runtime effect
is painted, we will overflow a byte very rapidly.
Change-Id: Ic094af2a81e590488bf90b60492b004e9135d4a2
Bug: skia:13405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551843
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: Ie2166fefe6ffa431b17590c53bf22a8725376938
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551885
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Without this define, I was seeing a compile error in SkKeyHelpers.cpp
Change-Id: I4d4a804f11fdd67e69bac4c7a6d0d686bebd8922
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/552076
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
SkArenaAlloc uses new char[allocSize] to obtain new memory blocks.
Changes this to sk_malloc_throw which does not zero allocated
memory.
I don't think anything relies on the memory being zeroed. If
this CL makes it through all the *SANs I think this CL will
be ok in the field.
Change-Id: Ia564313a501dd7c5b34ac6a9bde71fab1d76be76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551880
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Replaces Device::recordDraw() with a static ChooseRenderer() function
that is closer to the final destination for renderer selection, and
provides a helper function drawClipShape() for ClipStack to use instead
of having it append to the DrawContext directly.
The renderer choosing now happens earlier in Device::drawGeometry()
so that it is available to modify bounds/draw-order calculations if
it requires coverage AA.
Bug: skia:12787
Change-Id: Ia12db71bcd3ab6fbecfc72c79f06a2597388eafd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550699
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Nicolette Prevost <nicolettep@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This is a reland of commit fb13c18ddf
Bazel 5.2.0 was fine, it was the fact that our toolchain
was not erroring when clang failed that was an issue.
We were always returning error code 0, but Bazel 5.2.0
was correctly identifying that we hadn't produced the
promised build artifacts.
Original change's description:
> [bazel] Update to v5.2.0
>
> The primary change we are looking for is
> "Add support for .ar archives (and .deb files)
> https://github.com/bazelbuild/bazel/pull/15218"
>
> https://blog.bazel.build/2022/06/08/bazel-5.2.html
>
> In theory, we should be able to trigger Linux RBE
> compilations (e.g. IWYU) from Mac with this change.
>
> Change-Id: I217406d21fd55507e090c4bb5f79c796230717e6
> Bug: skia:12541
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549659
> Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Bug: skia:12541
Change-Id: I160b5802adc232d5cf7f7d05b20d5eabbb3d5102
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551841
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Note: in variedtext.cpp:66 changed static_assert to SkASSERT.
Change-Id: I853a2e5563c90c9dde5d6ba5443cc73b664b493d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551876
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: Id58bf93098fa62f621614937a2d1524790f73f58
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551708
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
In order to support a larger range for stop offsets in variable fonts, I
plan to resize the stop_offset field to 16.16 in FreeType. Prepare Skia
for handling that change.
Bug: chromium:1311241
Change-Id: I1da46a3e4c79f2f3d091e65005c19d16dc382669
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551638
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Dominik Röttsches <drott@google.com>
Enable passing in a feature-test function that returns a Boolean
with the intention of being able to use it in FreeType library
initialisation.
See also:
https://gitlab.freedesktop.org/freetype/freetype/-/merge_requests/168
Bug: skia:13448
Change-Id: I6c985201552b54f2fa99e911265b7303ff8b2897
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551637
Commit-Queue: Dominik Röttsches <drott@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
In MtlGraphicsPipeline, get_sksl_fs() will return '{}' if
SK_SUPPORT_GPU is false, which means it won't work in Graphite-only
builds. Making the related code Graphite-only fixes the link errors.
Change-Id: Ia6e9a99ad506c637b43a0830941f775ec0678b03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550713
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This combines the BruteForceManager and Gridmanager, starting with
brute force when it's fastest for low N and then transitioning to less
accurate grid that scales for larger N.
Also updates the set of benchmarks to run based on more reasonable
configs (e.g. having a reasonable level of accuracy to be worth
considering).
Updates Device to use the HybridBoundsManager with brute force up to
64 draws, and then a grid configured to make 16x16 pixel cells. My
guess is we will see a mix of perf regressions and improvements with
this. The existing use of the NaiveBoundsManager had negligible CPU
cost but disallowed all re-ordering. The brute force and grid
managers will add CPU cost but enable re-ordering, which shows up
as shorter command buffers (e.g. 17k commands vs. 28k commands in the
motionmark suits benchmark). However, because we don't have SSBOs
there still isn't as much batching that would let the GPU take
advantage of this re-ordering so I'm not sure how visible the wins
will be yet.
Bug: skia:13201, skia:12787
Change-Id: Iad58fccab45def5f702a30860e063669424dfcf2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550518
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Nicolette Prevost <nicolettep@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:13427
Change-Id: I7a0833e19c7d97157cb18b9ad9a11f2d25427b2b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551836
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Added changes for public.bzl
This is a reland of commit 4a375fe213
Original change's description:
> Move SkSubRun to src/text
>
> Change-Id: I5c1040b8236dc792de20495a3fea3c0be6e31c20
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549847
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I86e342a416a3c97bf972c496b93b37e35cd09c19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550496
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
