Fixed issues with resolving URL type 26/87126/5
authorKimmo Hoikka <kimmo.hoikka@samsung.com>
Tue, 6 Sep 2016 11:14:48 +0000 (12:14 +0100)
committerKimmo Hoikka <kimmo.hoikka@samsung.com>
Tue, 6 Sep 2016 12:32:30 +0000 (13:32 +0100)
Issues:
- string was parsed multiple times
- substr was used (which creates copy)
- batching was checked even for Npatch and SVG
Solved by implementing a single pass early out parser to process the URL and return the type

Change-Id: Ia0cbe51a6969b0dd26a35abde1ed2de82bf863f8

automated-tests/src/dali-toolkit-internal/CMakeLists.txt
automated-tests/src/dali-toolkit-internal/utc-Dali-VisualFactoryResolveUrl.cpp [new file with mode: 0644]
dali-toolkit/internal/visuals/svg/svg-visual.cpp
dali-toolkit/internal/visuals/svg/svg-visual.h
dali-toolkit/internal/visuals/visual-factory-impl.cpp
dali-toolkit/internal/visuals/visual-factory-resolve-url.h [new file with mode: 0644]

index bb55422..d2316e8 100644 (file)
@@ -18,6 +18,7 @@ SET(TC_SOURCES
  utc-Dali-VisualModel.cpp
  utc-Dali-Text-Layout.cpp
  utc-Dali-Text-Controller.cpp
+ utc-Dali-VisualFactoryResolveUrl.cpp
 )
 
 # Append list of test harness files (Won't get parsed for test cases)
diff --git a/automated-tests/src/dali-toolkit-internal/utc-Dali-VisualFactoryResolveUrl.cpp b/automated-tests/src/dali-toolkit-internal/utc-Dali-VisualFactoryResolveUrl.cpp
new file mode 100644 (file)
index 0000000..f859b74
--- /dev/null
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include <iostream>
+
+#include <stdlib.h>
+
+#include <dali-toolkit-test-suite-utils.h>
+#include <dali-toolkit/internal/visuals/visual-factory-resolve-url.h>
+
+using namespace Dali::Toolkit::Internal;
+
+int UtcDaliResolveUrlRegularImage(void)
+{
+  tet_infoline( "UtcDaliResolveUrl REGULAR_IMAGE" );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("foobar.jpeg"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("foobar.gif"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("foobar.PNG"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("foobar.Png123"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("foobar.Png1.23"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType(""), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType(" "), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("."), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("9"), TEST_LOCATION );
+
+  END_TEST;
+}
+
+int UtcDaliResolveUrlSvg(void)
+{
+  tet_infoline( "UtcDaliResolveUrl SVG" );
+
+  DALI_TEST_EQUALS( UrlType::SVG, ResolveUrlType("foobar.svg"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::SVG, ResolveUrlType("foobar.svg.svg"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::SVG, ResolveUrlType("foobar.svG"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::SVG, ResolveUrlType("foobar.SVG"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::SVG, ResolveUrlType(".SvG"), TEST_LOCATION );
+
+  // SVGs aren't N-patch
+  DALI_TEST_EQUALS( UrlType::SVG, ResolveUrlType("foobar.9.svg"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("svg.png"), TEST_LOCATION );
+
+  // maybe controversial, but for now we expect the suffix to be exactly .svg
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("svg.svg1"), TEST_LOCATION );
+
+  END_TEST;
+}
+
+int UtcDaliResolveUrlNPatch(void)
+{
+  tet_infoline( "UtcDaliResolveUrl N_PATCH" );
+
+  DALI_TEST_EQUALS( UrlType::N_PATCH, ResolveUrlType("foobar.9.gif"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::N_PATCH, ResolveUrlType("foobar.#.png"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::N_PATCH, ResolveUrlType("foobar.9.9.bmp"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::N_PATCH, ResolveUrlType("foobar.9.9.jpg[]=$$"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::N_PATCH, ResolveUrlType("foobar.9.#.#.9.wbpm123"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("svg.##.png"), TEST_LOCATION );
+
+  DALI_TEST_EQUALS( UrlType::REGULAR_IMAGE, ResolveUrlType("svg.99.jpeg"), TEST_LOCATION );
+
+  END_TEST;
+}
index 4179d4d..569c696 100644 (file)
@@ -70,11 +70,6 @@ SvgVisual::~SvgVisual()
   }
 }
 
-bool SvgVisual::IsSvgUrl( const std::string& url )
-{
-  return url.substr( url.find_last_of(".") + 1 ) == "svg";
-}
-
 void SvgVisual::DoInitialize( Actor& actor, const Property::Map& propertyMap )
 {
   Property::Value* imageURLValue = propertyMap.Find( Toolkit::ImageVisual::Property::URL, IMAGE_URL_NAME );
index d841621..c583035 100644 (file)
@@ -95,14 +95,6 @@ protected:
 public:
 
   /**
-   * @brief Helper method to determine whether the url indicate that it is a svg image.
-   *
-   * @param [in] url The URL of the image file.
-   * @return true if it is a svg image
-   */
-  static bool IsSvgUrl( const std::string& url );
-
-  /**
    * @brief Sets the svg image of this visual to the resource at imageUrl
    * The visual will parse the svg image once it is set.
    * And rasterize it into BufferImage synchronously when the associated actor is put on stage, and destroy the BufferImage when it is off stage
index e35c98b..bcc9acd 100644 (file)
@@ -40,6 +40,7 @@
 #include <dali-toolkit/internal/visuals/svg/svg-visual.h>
 #include <dali-toolkit/internal/visuals/wireframe/wireframe-visual.h>
 #include <dali-toolkit/internal/visuals/visual-factory-cache.h>
+#include <dali-toolkit/internal/visuals/visual-factory-resolve-url.h>
 #include <dali-toolkit/internal/visuals/visual-string-constants.h>
 
 namespace Dali
@@ -136,28 +137,33 @@ Toolkit::Visual::Base VisualFactory::CreateVisual( const Property::Map& property
       std::string imageUrl;
       if( imageURLValue && imageURLValue->Get( imageUrl ) )
       {
-        Property::Value* batchingEnabledValue = propertyMap.Find( Toolkit::ImageVisual::Property::BATCHING_ENABLED, BATCHING_ENABLED );
-        if( batchingEnabledValue  )
-        {
-          bool batchingEnabled( false );
-          batchingEnabledValue->Get( batchingEnabled );
-          if( batchingEnabled )
-          {
-            visualPtr = new BatchImageVisual( *( mFactoryCache.Get() ) );
-            break;
-          }
-        }
-        else if( NinePatchImage::IsNinePatchUrl( imageUrl ) )
+        // first resolve url type to know which visual to create
+        UrlType::Type type = ResolveUrlType( imageUrl );
+        if( UrlType::N_PATCH == type )
         {
           visualPtr = new NPatchVisual( *( mFactoryCache.Get() ) );
         }
-        else if( SvgVisual::IsSvgUrl( imageUrl ) )
+        else if( UrlType::SVG == type )
         {
           visualPtr = new SvgVisual( *( mFactoryCache.Get() ) );
         }
-        else
+        else // Regular image
         {
-          visualPtr = new ImageVisual( *( mFactoryCache.Get() ) );
+          Property::Value* batchingEnabledValue = propertyMap.Find( Toolkit::ImageVisual::Property::BATCHING_ENABLED, BATCHING_ENABLED );
+          if( batchingEnabledValue  )
+          {
+            bool batchingEnabled( false );
+            batchingEnabledValue->Get( batchingEnabled );
+            if( batchingEnabled )
+            {
+              visualPtr = new BatchImageVisual( *( mFactoryCache.Get() ) );
+              break;
+            }
+          }
+          else
+          {
+            visualPtr = new ImageVisual( *( mFactoryCache.Get() ) );
+          }
         }
       }
 
@@ -239,20 +245,22 @@ Toolkit::Visual::Base VisualFactory::CreateVisual( const std::string& url, Image
     return Toolkit::Visual::Base( new WireframeVisual( *( mFactoryCache.Get() ) ) );
   }
 
-  if( NinePatchImage::IsNinePatchUrl( url ) )
+  // first resolve url type to know which visual to create
+  UrlType::Type type = ResolveUrlType( url );
+  if( UrlType::N_PATCH == type )
   {
     NPatchVisual* visualPtr = new NPatchVisual( *( mFactoryCache.Get() ) );
     visualPtr->SetImage( url );
 
     return Toolkit::Visual::Base( visualPtr );
   }
-  else if( SvgVisual::IsSvgUrl( url ) )
+  else if( UrlType::SVG == type )
   {
     SvgVisual* visualPtr = new SvgVisual( *( mFactoryCache.Get() ) );
     visualPtr->SetImage( url, size );
     return Toolkit::Visual::Base( visualPtr );
   }
-  else
+  else // Regular image
   {
     ImageVisual* visualPtr = new ImageVisual( *( mFactoryCache.Get() ));
     Actor actor;
diff --git a/dali-toolkit/internal/visuals/visual-factory-resolve-url.h b/dali-toolkit/internal/visuals/visual-factory-resolve-url.h
new file mode 100644 (file)
index 0000000..1d3ffe3
--- /dev/null
@@ -0,0 +1,121 @@
+#ifndef DALI_TOOLKIT_VISUAL_FACTORY_URL_RESOLVE_H
+#define DALI_TOOLKIT_VISUAL_FACTORY_URL_RESOLVE_H
+
+/*
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <ctype.h>
+
+namespace Dali
+{
+
+namespace Toolkit
+{
+
+namespace Internal
+{
+
+namespace UrlType
+{
+  /**
+   * The type of the URL based on the string contents
+   */
+  enum Type
+  {
+    REGULAR_IMAGE,
+    N_PATCH,
+    SVG
+  };
+}
+
+/**
+ * Helper to resolve URL type from the string
+ * @param[in] url to check
+ * @return UrlType
+ */
+inline UrlType::Type ResolveUrlType( const std::string& url )
+{
+  // if only one char in string, can only be regular image
+  const std::size_t count = url.size();
+  if( count > 0 )
+  {
+    // parsing from the end for better chance of early outs
+    enum { SUFFIX, HASH, HASH_DOT } state = SUFFIX;
+    char SVG[ 4 ] = { 'g', 'v', 's', '.' };
+    unsigned int svgScore = 0;
+    int index = count;
+    while( --index >= 0 )
+    {
+      const char currentChar = url[ index ];
+      const std::size_t offsetFromEnd = count - index - 1u;
+      if( ( offsetFromEnd < sizeof(SVG) )&&( tolower( currentChar ) == SVG[ offsetFromEnd ] ) )
+      {
+        // early out if SVG as can't be used in N patch for now
+        if( ++svgScore == sizeof(SVG) )
+        {
+          return UrlType::SVG;
+        }
+      }
+      switch( state )
+      {
+        case SUFFIX:
+        {
+          if( '.' == currentChar )
+          {
+            state = HASH;
+          }
+          break;
+        }
+        case HASH:
+        {
+          if( ( '#' == currentChar ) || ( '9' == currentChar ) )
+          {
+            state = HASH_DOT;
+          }
+          else
+          {
+            // early out, not a valid N/9-patch URL
+            return UrlType::REGULAR_IMAGE;
+          }
+          break;
+        }
+        case HASH_DOT:
+        {
+          if( '.' == currentChar )
+          {
+            return UrlType::N_PATCH;
+          }
+          else
+          {
+            // early out, not a valid N/9-patch URL
+            return UrlType::REGULAR_IMAGE;
+          }
+          break;
+        }
+      }
+    }
+  }
+  // if we got here it is a regular image
+  return UrlType::REGULAR_IMAGE;
+}
+
+} // namespace Internal
+
+} // namespace Toolkit
+
+} // namespace Dali
+
+#endif /* DALI_TOOLKIT_VISUAL_FACTORY_URL_RESOLVE_H */