Add error check for float parsing and fix tests (1e67f089) · Commits · e / os / android_frameworks_base

core/jni/android_util_PathParser.cpp

+8 −3

Original line number	Original line	Diff line number	Diff line
	@@ -19,6 +19,7 @@
	#include <PathParser.h>		#include <PathParser.h>
	#include <SkPath.h>		#include <SkPath.h>

			#include <android/log.h>
	#include "core_jni_helpers.h"		#include "core_jni_helpers.h"

	namespace android {		namespace android {
	@@ -27,10 +28,14 @@ static bool parseStringForPath(JNIEnv* env, jobject, jlong skPathHandle, jstring
	jint strLength) {		jint strLength) {
	const char* pathString = env->GetStringUTFChars(inputPathStr, NULL);		const char* pathString = env->GetStringUTFChars(inputPathStr, NULL);
	SkPath* skPath = reinterpret_cast<SkPath*>(skPathHandle);		SkPath* skPath = reinterpret_cast<SkPath*>(skPathHandle);
	bool hasValidData = android::uirenderer::PathParser::parseStringForSkPath(skPath, pathString,
	strLength);		android::uirenderer::PathParser::ParseResult result;
			android::uirenderer::PathParser::parseStringForSkPath(skPath, &result, pathString, strLength);
	env->ReleaseStringUTFChars(inputPathStr, pathString);		env->ReleaseStringUTFChars(inputPathStr, pathString);
	return hasValidData;		if (result.failureOccurred) {
			ALOGE(result.failureMessage.c_str());
			}
			return !result.failureOccurred;
	}		}

	static const JNINativeMethod gMethods[] = {		static const JNINativeMethod gMethods[] = {

libs/hwui/PathParser.cpp

+52 −22

Original line number	Original line	Diff line number	Diff line
	@@ -18,6 +18,7 @@

	#include "jni.h"		#include "jni.h"

			#include <errno.h>
	#include <utils/Log.h>		#include <utils/Log.h>
	#include <sstream>		#include <sstream>
	#include <stdlib.h>		#include <stdlib.h>
	@@ -97,14 +98,31 @@ static void extract(int* outEndPosition, bool* outEndWithNegOrDot, const char* s
	*outEndPosition = currentIndex;		*outEndPosition = currentIndex;
	}		}

			static float parseFloat(PathParser::ParseResult* result, const char* startPtr, size_t expectedLength) {
			char* endPtr = NULL;
			float currentValue = strtof(startPtr, &endPtr);
			if ((currentValue == HUGE_VALF \|\| currentValue == -HUGE_VALF) && errno == ERANGE) {
			result->failureOccurred = true;
			result->failureMessage = "Float out of range: ";
			result->failureMessage.append(startPtr, expectedLength);
			}
			if (currentValue == 0 && endPtr == startPtr) {
			// No conversion is done.
			result->failureOccurred = true;
			result->failureMessage = "Float format error when parsing: ";
			result->failureMessage.append(startPtr, expectedLength);
			}
			return currentValue;
			}

	/**		/**
	* Parse the floats in the string.		* Parse the floats in the string.
	* This is an optimized version of parseFloat(s.split(",\|\\s"));
	*		*
	* @param s the string containing a command and list of floats		* @param s the string containing a command and list of floats
	* @return array of floats		* @return true on success
	*/		*/
	static void getFloats(std::vector<float>* outPoints, const char* pathStr, int start, int end) {		static void getFloats(std::vector<float>* outPoints, PathParser::ParseResult* result,
			const char* pathStr, int start, int end) {

	if (pathStr[start] == 'z' \|\| pathStr[start] == 'Z') {		if (pathStr[start] == 'z' \|\| pathStr[start] == 'Z') {
	return;		return;
	@@ -120,7 +138,12 @@ static void getFloats(std::vector<float>* outPoints, const char* pathStr, int st
	extract(&endPosition, &endWithNegOrDot, pathStr, startPosition, end);		extract(&endPosition, &endWithNegOrDot, pathStr, startPosition, end);

	if (startPosition < endPosition) {		if (startPosition < endPosition) {
	outPoints->push_back(strtof(&pathStr[startPosition], NULL));		float currentValue = parseFloat(result, &pathStr[startPosition],
			end - startPosition);
			if (result->failureOccurred) {
			return;
			}
			outPoints->push_back(currentValue);
	}		}

	if (endWithNegOrDot) {		if (endWithNegOrDot) {
	@@ -130,10 +153,14 @@ static void getFloats(std::vector<float>* outPoints, const char* pathStr, int st
	startPosition = endPosition + 1;		startPosition = endPosition + 1;
	}		}
	}		}
			return;
	}		}

	void PathParser::getPathDataFromString(PathData* data, const char* pathStr, size_t strLen) {		void PathParser::getPathDataFromString(PathData* data, ParseResult* result,
			const char* pathStr, size_t strLen) {
	if (pathStr == NULL) {		if (pathStr == NULL) {
			result->failureOccurred = true;
			result->failureMessage = "Path string cannot be NULL.";
	return;		return;
	}		}

	@@ -143,7 +170,10 @@ void PathParser::getPathDataFromString(PathData* data, const char* pathStr, size
	while (end < strLen) {		while (end < strLen) {
	end = nextStart(pathStr, strLen, end);		end = nextStart(pathStr, strLen, end);
	std::vector<float> points;		std::vector<float> points;
	getFloats(&points, pathStr, start, end);		getFloats(&points, result, pathStr, start, end);
			if (result->failureOccurred) {
			return;
			}
	data->verbs.push_back(pathStr[start]);		data->verbs.push_back(pathStr[start]);
	data->verbSizes.push_back(points.size());		data->verbSizes.push_back(points.size());
	data->points.insert(data->points.end(), points.begin(), points.end());		data->points.insert(data->points.end(), points.begin(), points.end());
	@@ -151,16 +181,11 @@ void PathParser::getPathDataFromString(PathData* data, const char* pathStr, size
	end++;		end++;
	}		}

	if ((end - start) == 1 && pathStr[start] != '\0') {		if ((end - start) == 1 && start < strLen) {
	data->verbs.push_back(pathStr[start]);		data->verbs.push_back(pathStr[start]);
	data->verbSizes.push_back(0);		data->verbSizes.push_back(0);
	}		}
			return;
	int i = 0;
	while(pathStr[i] != '\0') {
	i++;
	}

	}		}

	void PathParser::dump(const PathData& data) {		void PathParser::dump(const PathData& data) {
	@@ -169,6 +194,7 @@ void PathParser::dump(const PathData& data) {
	for (size_t i = 0; i < data.verbs.size(); i++) {		for (size_t i = 0; i < data.verbs.size(); i++) {
	std::ostringstream os;		std::ostringstream os;
	os << data.verbs[i];		os << data.verbs[i];
			os << ", verb size: " << data.verbSizes[i];
	for (size_t j = 0; j < data.verbSizes[i]; j++) {		for (size_t j = 0; j < data.verbSizes[i]; j++) {
	os << " " << data.points[start + j];		os << " " << data.points[start + j];
	}		}
	@@ -183,16 +209,20 @@ void PathParser::dump(const PathData& data) {
	ALOGD("points are : %s", os.str().c_str());		ALOGD("points are : %s", os.str().c_str());
	}		}

	bool PathParser::parseStringForSkPath(SkPath* skPath, const char* pathStr, size_t strLen) {		void PathParser::parseStringForSkPath(SkPath* skPath, ParseResult* result, const char* pathStr, size_t strLen) {
	PathData pathData;		PathData pathData;
	getPathDataFromString(&pathData, pathStr, strLen);		getPathDataFromString(&pathData, result, pathStr, strLen);
			if (result->failureOccurred) {
			return;
			}
	// Check if there is valid data coming out of parsing the string.		// Check if there is valid data coming out of parsing the string.
	if (pathData.verbs.size() == 0) {		if (pathData.verbs.size() == 0) {
	return false;		result->failureOccurred = true;
			result->failureMessage = "No verbs found in the string for pathData";
			return;
	}		}
	VectorDrawablePath::verbsToPath(skPath, &pathData);		VectorDrawablePath::verbsToPath(skPath, &pathData);
	return true;		return;
	}		}

	}; // namespace uirenderer		}; // namespace uirenderer

libs/hwui/PathParser.h

+11 −3

Original line number	Original line	Diff line number	Diff line
	@@ -23,17 +23,25 @@
	#include <android/log.h>		#include <android/log.h>
	#include <cutils/compiler.h>		#include <cutils/compiler.h>

			#include <string>

	namespace android {		namespace android {
	namespace uirenderer {		namespace uirenderer {


	class PathParser {		class PathParser {
	public:		public:
			struct ANDROID_API ParseResult {
			bool failureOccurred = false;
			std::string failureMessage;
			};
	/**		/**
	* Parse the string literal and create a Skia Path. Return true on success.		* Parse the string literal and create a Skia Path. Return true on success.
	*/		*/
	ANDROID_API static bool parseStringForSkPath(SkPath* outPath, const char* pathStr,		ANDROID_API static void parseStringForSkPath(SkPath* outPath, ParseResult* result,
	size_t strLength);		const char* pathStr, size_t strLength);
	static void getPathDataFromString(PathData* outData, const char* pathStr, size_t strLength);		static void getPathDataFromString(PathData* outData, ParseResult* result,
			const char* pathStr, size_t strLength);
	static void dump(const PathData& data);		static void dump(const PathData& data);
	};		};

libs/hwui/VectorDrawablePath.cpp

+8 −5

Original line number	Original line	Diff line number	Diff line
	@@ -37,9 +37,12 @@ public:
	};		};

	VectorDrawablePath::VectorDrawablePath(const char* pathStr, size_t strLength) {		VectorDrawablePath::VectorDrawablePath(const char* pathStr, size_t strLength) {
	PathParser::getPathDataFromString(&mData, pathStr, strLength);		PathParser::ParseResult result;
			PathParser::getPathDataFromString(&mData, &result, pathStr, strLength);
			if (!result.failureOccurred) {
	verbsToPath(&mSkPath, &mData);		verbsToPath(&mSkPath, &mData);
	}		}
			}

	VectorDrawablePath::VectorDrawablePath(const PathData& data) {		VectorDrawablePath::VectorDrawablePath(const PathData& data) {
	mData = data;		mData = data;
	@@ -80,7 +83,7 @@ void VectorDrawablePath::verbsToPath(SkPath* outPath, const PathData* data) {
	for (unsigned int i = 0; i < data->verbs.size(); i++) {		for (unsigned int i = 0; i < data->verbs.size(); i++) {
	size_t verbSize = data->verbSizes[i];		size_t verbSize = data->verbSizes[i];
	resolver.addCommand(outPath, previousCommand, data->verbs[i], &data->points, start,		resolver.addCommand(outPath, previousCommand, data->verbs[i], &data->points, start,
	start + verbSize - 1u);		start + verbSize);
	previousCommand = data->verbs[i];		previousCommand = data->verbs[i];
	start += verbSize;		start += verbSize;
	}		}
	@@ -254,7 +257,7 @@ static void drawArc(SkPath* p,
	arcToBezier(p, cx, cy, a, b, x0, y0, thetaD, eta0, sweep);		arcToBezier(p, cx, cy, a, b, x0, y0, thetaD, eta0, sweep);
	}		}

			// Use the given verb, and points in the range [start, end) to insert a command into the SkPath.
	void PathResolver::addCommand(SkPath* outPath, char previousCmd,		void PathResolver::addCommand(SkPath* outPath, char previousCmd,
	char cmd, const std::vector<float>* points, size_t start, size_t end) {		char cmd, const std::vector<float>* points, size_t start, size_t end) {

	@@ -305,7 +308,7 @@ void PathResolver::addCommand(SkPath* outPath, char previousCmd,
	break;		break;
	}		}

	for (unsigned int k = start; k <= end; k += incr) {		for (unsigned int k = start; k < end; k += incr) {
	switch (cmd) {		switch (cmd) {
	case 'm': // moveto - Start a new sub-path (relative)		case 'm': // moveto - Start a new sub-path (relative)
	currentX += points->at(k + 0);		currentX += points->at(k + 0);

libs/hwui/microbench/PathParserBench.cpp

+2 −1

Original line number	Original line	Diff line number	Diff line
	@@ -28,9 +28,10 @@ void BM_PathParser_parseStringPath::Run(int iter) {
	const char* pathString = "M 1 1 m 2 2, l 3 3 L 3 3 H 4 h4 V5 v5, Q6 6 6 6 q 6 6 6 6t 7 7 T 7 7 C 8 8 8 8 8 8 c 8 8 8 8 8 8 S 9 9 9 9 s 9 9 9 9 A 10 10 0 1 1 10 10 a 10 10 0 1 1 10 10";		const char* pathString = "M 1 1 m 2 2, l 3 3 L 3 3 H 4 h4 V5 v5, Q6 6 6 6 q 6 6 6 6t 7 7 T 7 7 C 8 8 8 8 8 8 c 8 8 8 8 8 8 S 9 9 9 9 s 9 9 9 9 A 10 10 0 1 1 10 10 a 10 10 0 1 1 10 10";
	SkPath skPath;		SkPath skPath;
	size_t length = strlen(pathString);		size_t length = strlen(pathString);
			PathParser::ParseResult result;
	StartBenchmarkTiming();		StartBenchmarkTiming();
	for (int i = 0; i < iter; i++) {		for (int i = 0; i < iter; i++) {
	PathParser::parseStringForSkPath(&skPath, pathString, length);		PathParser::parseStringForSkPath(&skPath, &result, pathString, length);
	}		}
	StopBenchmarkTiming();		StopBenchmarkTiming();
	}		}