Reland "Redo how extra emit code flushing operates" with type fix.
This reverts commit d0440195d5cc049062238d02b8a962ae49c7f4ff.
Reason for revert: Fixes size_t -> int that was triggering ASAN failures.
Original change's description:
> Revert "Redo how extra emit code flushing operates"
>
> This reverts commit 9b8181b05a84e7dd24234c46c87d0bb2c73a7c08.
>
> Reason for revert: <INSERT REASONING HERE>
>
> Original change's description:
> > Redo how extra emit code flushing operates
> >
> > The previous implementation of flushEmittedCode(), that flushed on
> > demand when a process() was encountered, was brittle and susceptible to
> > mangling the expected sksl when fOut was modified outside of its
> > control. Given that writeFunction() and generateCode() in the parent
> > class all do this, it's possible to generate a simple SkSL snippet that
> > would generate a CPP file that builds invalid final SkSL:
> >
> > ```
> > in fragmentProcessor child;
> > bool someGlobalVar = ...;
> > void main() {
> > if (someGlobalVar) {
> > sk_OutColor = process(child, sk_InColor);
> > } else {
> > sk_OutColor = half4(1);
> > }
> > }
> > ```
> >
> > The CPP generated code *should* insert 'bool someGlobalVar' at the start
> > but because of the early flush from the child process and because of
> > how fOut was overwritten, someGlobalVar's declaration is put into a
> > stream that is not visible to the flush and ends up being inserted into
> > the output sksl in an incorrect location (namely after the if condition
> > that depends on it).
> >
> > This CL updates the extra emitted code logic to support multiple blocks
> > of extra CPP code. When a flush point occurs in SkSL writing, a special
> > token is inserted into the SkSL and a new CPP code buffer is associated
> > with that token. Then once all of the SkSL is accumulated into the root
> > output stream, it is processed into sections for each extra CPP block.
> > Special logic is done so that the SkSL that is emitted before the next
> > CPP block terminates at the end of the last valid statement before the
> > special token.
> >
> > A unit test demonstrating this failure condition is added to SkSLFPTest
> > and the CL properly passes. Since this bug did not trigger on existing
> > .fp files, the updated generator does not modify the generated FPs.
> >
> > Bug: skia:
> > Change-Id: Ib74911942080f1b964159807a06805bc52898789
> > Reviewed-on: https://skia-review.googlesource.com/152321
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
>
> TBR=ethannicholas@google.com,michaelludwig@google.com
>
> Change-Id: Id0f908453b596873f43b86a1c14eed48b2474a76
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:
> Reviewed-on: https://skia-review.googlesource.com/152660
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=ethannicholas@google.com,michaelludwig@google.com
Change-Id: I3ccf2fee6ef96c6102dbe1c2c2ef6c14a701b8fd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/152663
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/sksl/SkSLCPPCodeGenerator.h b/src/sksl/SkSLCPPCodeGenerator.h
index 6c67bad..4b62fc6 100644
--- a/src/sksl/SkSLCPPCodeGenerator.h
+++ b/src/sksl/SkSLCPPCodeGenerator.h
@@ -23,21 +23,6 @@
bool generateCode() override;
private:
- // When inside writeEmitCode(), certain SkSL elements need to control
- // when fragBuilder->codeAppendf is added to the function block. This
- // takes all completed statements in the SkSL buffer, and their corresponding
- // format args, and writes them into the emitCode()'s statement block
- // using writeCodeAppend().
- //
- // This control is necessary for handling special functions in SkSL, like
- // process(), which need to intermix the current FP's SkSL with that of
- // an emitted child.
- //
- // :forceAll - If false, only the completed statements (terminated by ;),
- // will be flushed and the sksl buffer will be set to any partial
- // statements that remain. If true, everything is flushed, regardless.
- void flushEmittedCode(bool forceAll = false);
-
void writef(const char* s, va_list va) SKSL_PRINTF_LIKE(2, 0);
void writef(const char* s, ...) SKSL_PRINTF_LIKE(2, 3);
@@ -103,19 +88,36 @@
void writeTest();
- // If the returned C++ is included in the generated code, then the variable
- // name stored in cppVar will refer to a valid SkString that matches the
- // Expression. Successful returns leave the output buffer (and related state)
- // unmodified.
+ // If the returned C++ is included in the generated code, then the variable name stored in
+ // cppVar will refer to a valid SkString that matches the Expression. Successful returns leave
+ // the output buffer (and related state) unmodified.
//
- // In the simplest cases, this will return "SkString {cppVar}(\"{e}\");",
- // while more advanced cases will properly insert format arguments.
+ // In the simplest cases, this will return "SkString {cppVar}(\"{e}\");", while more advanced
+ // cases will properly insert format arguments.
String convertSKSLExpressionToCPP(const Expression& e, const String& cppVar);
+ // Process accumulated sksl to split it into appended code sections, properly interleaved with
+ // the extra emit code blocks, based on statement/block locations and the inserted tokens
+ // from newExtraEmitCodeBlock(). It is necessary to split the sksl after the program has been
+ // fully walked since many elements redirect fOut to simultaneously build header sections and
+ // bodies that are then concatenated; due to this it is not possible to split the sksl emission
+ // on the fly.
+ void flushEmittedCode();
+
+ // Start a new extra emit code block for accumulating C++ code. This will insert a token into
+ // the sksl stream to mark the fence between previous complete sksl statements and where the
+ // C++ code added to the new block will be added to emitCode(). These tokens are removed by
+ // flushEmittedCode() as it consumes them before passing pure sksl to writeCodeAppend().
+ void newExtraEmitCodeBlock();
+
+ // Append CPP code to the current extra emit code block.
+ void addExtraEmitCodeLine(const String& toAppend);
+
String fName;
String fFullName;
SectionAndParameterHelper fSectionAndParameterHelper;
- String fExtraEmitCodeCode;
+ std::vector<String> fExtraEmitCodeBlocks;
+
std::vector<String> fFormatArgs;
std::set<int> fWrittenTransformedCoords;
// if true, we are writing a C++ expression instead of a GLSL expression