Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(308)

Unified Diff: gpu/command_buffer/service/shader_manager.cc

Issue 8404029: Strip comments from shader before checking for invalid characters (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 9 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: gpu/command_buffer/service/shader_manager.cc
diff --git a/gpu/command_buffer/service/shader_manager.cc b/gpu/command_buffer/service/shader_manager.cc
index b230581d6d1eb009fe6762e6e9c2229857b844a5..caac49431836be4087a043eed6283cbbc53e3090 100644
--- a/gpu/command_buffer/service/shader_manager.cc
+++ b/gpu/command_buffer/service/shader_manager.cc
@@ -4,6 +4,7 @@
#include "gpu/command_buffer/service/shader_manager.h"
#include "base/logging.h"
+#include "base/string_util.h"
namespace gpu {
namespace gles2 {
@@ -151,6 +152,192 @@ void ShaderManager::UnuseShader(ShaderManager::ShaderInfo* info) {
RemoveShaderInfoIfUnused(info);
}
+namespace {
+
+// Strips comments from shader text. This allows non-ASCII characters
+// to be used in comments without potentially breaking OpenGL
+// implementations not expecting characters outside the GLSL ES set.
+class Stripper {
+ public:
+ Stripper(const std::string& str)
+ : parse_state_(kBeginningOfLine)
+ , source_string_(str)
+ , length_(str.length())
+ , position_(0) {
+ Parse();
+ }
+
+ const std::string& result() {
+ return result_;
+ }
+
+ private:
+ bool HasMoreCharacters() {
+ return position_ < length_;
+ }
+
+ void Parse() {
+ while (HasMoreCharacters()) {
+ Process(Current());
+ // Process() might Advance the position.
+ if (HasMoreCharacters()) {
+ Advance();
+ }
+ }
+ }
+
+ void Process(char);
+
+ bool Peek(char* character) {
+ DCHECK(character);
+ if (position_ + 1 >= length_) {
+ return false;
+ }
+ *character = source_string_[position_ + 1];
+ return true;
+ }
+
+ char Current() {
+ DCHECK_LT(position_, length_);
+ return source_string_[position_];
+ }
+
+ void Advance() {
+ ++position_;
+ }
+
+ bool IsNewline(char character) {
+ // Don't attempt to canonicalize newline related characters.
+ return (character == '\n' || character == '\r');
+ }
+
+ void Emit(char character) {
+ result_.push_back(character);
+ }
+
+ enum ParseState {
+ // Have not seen an ASCII non-whitespace character yet on
+ // this line. Possible that we might see a preprocessor
+ // directive.
+ kBeginningOfLine,
+
+ // Have seen at least one ASCII non-whitespace character
+ // on this line.
+ kMiddleOfLine,
+
+ // Handling a preprocessor directive. Passes through all
+ // characters up to the end of the line. Disables comment
+ // processing.
+ kInPreprocessorDirective,
+
+ // Handling a single-line comment. The comment text is
+ // replaced with a single space.
+ kInSingleLineComment,
+
+ // Handling a multi-line comment. Newlines are passed
+ // through to preserve line numbers.
+ kInMultiLineComment
+ };
+
+ ParseState parse_state_;
+ std::string source_string_;
+ unsigned length_;
+ unsigned position_;
+ std::string result_;
+};
+
+void Stripper::Process(char c) {
+ if (IsNewline(c)) {
+ // No matter what state we are in, pass through newlines
+ // so we preserve line numbers.
+ Emit(c);
+
+ if (parse_state_ != kInMultiLineComment)
+ parse_state_ = kBeginningOfLine;
+
+ return;
+ }
+
+ char temp = 0;
+ switch (parse_state_) {
+ case kBeginningOfLine:
+ if (IsAsciiWhitespace(c)) {
+ Emit(c);
+ break;
+ }
+
+ if (c == '#') {
+ parse_state_ = kInPreprocessorDirective;
+ Emit(c);
+ break;
+ }
+
+ // Transition to normal state and re-handle character.
+ parse_state_ = kMiddleOfLine;
+ Process(c);
+ break;
+
+ case kMiddleOfLine:
+ if (c == '/' && Peek(&temp)) {
+ if (temp == '/') {
+ parse_state_ = kInSingleLineComment;
+ Emit(' ');
+ Advance();
+ break;
+ }
+
+ if (temp == '*') {
+ parse_state_ = kInMultiLineComment;
+ // Emit the comment start in case the user has
+ // an unclosed comment and we want to later
+ // signal an error.
+ Emit('/');
+ Emit('*');
+ Advance();
+ break;
+ }
+ }
+
+ Emit(c);
+ break;
+
+ case kInPreprocessorDirective:
+ // No matter what the character is, just pass it
+ // through. Do not Parse comments in this state. This
+ // might not be the right thing to do long term, but it
+ // should handle the #error preprocessor directive.
+ Emit(c);
+ break;
+
+ case kInSingleLineComment:
+ // The newline code at the top of this function takes care
+ // of resetting our state when we get out of the
+ // single-line comment. Swallow all other characters.
+ break;
+
+ case kInMultiLineComment:
+ if (c == '*' && Peek(&temp) && temp == '/') {
+ Emit('*');
+ Emit('/');
+ parse_state_ = kMiddleOfLine;
+ Advance();
+ break;
+ }
+
+ // Swallow all other characters. Unclear whether we may
+ // want or need to just Emit a space per character to try
+ // to preserve column numbers for debugging purposes.
+ break;
+ }
+}
+
+} // anonymous namespace
+
+std::string ShaderManager::StripComments(const std::string source) {
+ Stripper stripper(source);
+ return stripper.result();
+}
+
} // namespace gles2
} // namespace gpu

Powered by Google App Engine
This is Rietveld 408576698