[IMPROVEMENT/ACCEPTED] Xerces schema loading

[lucebac]

Hello,
I compiled the CEGUI source code from the repository and tried to run the samples but I got the following error (the render window indicated that the svg demo was loading):
"The filename supplied must be valid."

With the help of the log file I figured out that xerces wanted to load a not existing schema file (I think it's the one describing svg files, because there is no error on any other sample).

CEGUI.log:

Code: Select all

27/05/2014 21:02:16 (Std)    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
27/05/2014 21:02:16 (Std)    +                     Crazy Eddie's GUI System - Event log                    +
27/05/2014 21:02:16 (Std)    +                          (http://www.cegui.org.uk/)                         +
27/05/2014 21:02:16 (Std)    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

27/05/2014 21:02:16 (Std)    CEGUI::Logger singleton created. (06FC85C0)
27/05/2014 21:02:17 (Std)    
27/05/2014 21:02:17 (Std)    ********************************************************************************
27/05/2014 21:02:17 (Std)    * Important:                                                                   *
27/05/2014 21:02:17 (Std)    *     To get support at the CEGUI forums, you must post _at least_ the section *
27/05/2014 21:02:17 (Std)    *     of this log file indicated below.  Failure to do this will result in no  *
27/05/2014 21:02:17 (Std)    *     support being given; please do not waste our time.                       *
27/05/2014 21:02:17 (Std)    ********************************************************************************
27/05/2014 21:02:17 (Std)    ********************************************************************************
27/05/2014 21:02:17 (Std)    * -------- START OF ESSENTIAL SECTION TO BE POSTED ON THE FORUM       -------- *
27/05/2014 21:02:17 (Std)    ********************************************************************************
27/05/2014 21:02:17 (Std)    ---- Version: 9999.0.0 (Build: May 27 2014 Debug Microsoft Windows MSVC++ Great Scott! 32 bit) ----
27/05/2014 21:02:17 (Std)    ---- Renderer module is: CEGUI::OpenGL3Renderer - Official OpenGL 3.2 core based renderer module.  TextureTarget support enabled via FBO OGL 3.2 core implementation. ----
27/05/2014 21:02:17 (Std)    ---- XML Parser module is: CEGUI::XercesParser - Official Xerces-C++ based parser module for CEGUI ----
27/05/2014 21:02:17 (Std)    ---- Image Codec module is: SILLYImageCodec - Official SILLY based image codec ----
27/05/2014 21:02:17 (Std)    ---- Scripting module is: None ----
27/05/2014 21:02:17 (Std)    ********************************************************************************
27/05/2014 21:02:17 (Std)    * -------- END OF ESSENTIAL SECTION TO BE POSTED ON THE FORUM         -------- *
27/05/2014 21:02:17 (Std)    ********************************************************************************

--snip--   

27/05/2014 21:02:53 (Std)    [ImageManager] Started creation of Imageset from XML specification:
27/05/2014 21:02:53 (Std)    [ImageManager] ---- CEGUI Imageset name: SVGSampleImageset
27/05/2014 21:02:53 (Std)    [ImageManager] ---- Source image file: SVGSampleImage.svg
27/05/2014 21:02:53 (Std)    [ImageManager] ---- Source resource group: (Default)
27/05/2014 21:02:53 (Std)    XercesParser::initialiseSchema - Attempting to load schema from file ''.
27/05/2014 21:02:53 (Error)   CEGUI::InvalidRequestException in function 'void __thiscall CEGUI::DefaultResourceProvider::loadRawDataContainer(const class CEGUI::String &,class CEGUI::RawDataContainer &,const class CEGUI::String &)' (C:\Users\Luca\Documents\dev\cegui_tip\cegui\src\DefaultResourceProvider.cpp:56) : Filename supplied for data loading must be valid
27/05/2014 21:02:53 (Error)   ========== Start of Backtrace ==========
27/05/2014 21:02:53 (Error)   #0 CEGUI::Exception::Exception +0x326 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #1 CEGUI::InvalidRequestException::InvalidRequestException +0x79 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #2 CEGUI::DefaultResourceProvider::loadRawDataContainer +0xca (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #3 CEGUI::XercesParser::initialiseSchema +0x1db (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIXercesParser_d.dll)
27/05/2014 21:02:53 (Error)   #4 CEGUI::XercesParser::parseXML +0x7c (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIXercesParser_d.dll)
27/05/2014 21:02:53 (Error)   #5 CEGUI::XMLParser::parseXMLFile +0xb1 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #6 CEGUI::SVGData::loadFromFile +0x95 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #7 CEGUI::SVGData::SVGData +0x85 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #8 CEGUI::SVGDataManager::create +0x22a (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #9 CEGUI::ImageManager::retrieveImagesetSVGData +0x130 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #10 CEGUI::ImageManager::elementImagesetStart +0x459 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #11 CEGUI::ImageManager::elementStartLocal +0x91 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #12 CEGUI::ChainedXMLHandler::elementStart +0x6f (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #13 CEGUI::XercesHandler::startElement +0xbb (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIXercesParser_d.dll)
27/05/2014 21:02:53 (Error)   #14 xercesc_3_1::SAX2XMLReaderImpl::startElement +0x2ff (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\xerces-c_3_d.dll)
27/05/2014 21:02:53 (Error)   #15 xercesc_3_1::IGXMLScanner::scanStartTagNS +0x1105 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\xerces-c_3_d.dll)
27/05/2014 21:02:53 (Error)   #16 xercesc_3_1::IGXMLScanner::scanContent +0x19e (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\xerces-c_3_d.dll)
27/05/2014 21:02:53 (Error)   #17 xercesc_3_1::IGXMLScanner::scanDocument +0xf9 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\xerces-c_3_d.dll)
27/05/2014 21:02:53 (Error)   #18 xercesc_3_1::SAX2XMLReaderImpl::parse +0xc6 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\xerces-c_3_d.dll)
27/05/2014 21:02:53 (Error)   #19 CEGUI::XercesParser::doParse +0x97 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIXercesParser_d.dll)
27/05/2014 21:02:53 (Error)   #20 CEGUI::XercesParser::parseXML +0x8c (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIXercesParser_d.dll)
27/05/2014 21:02:53 (Error)   #21 CEGUI::XMLParser::parseXMLFile +0xb1 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #22 CEGUI::ImageManager::loadImageset +0x6e (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUIBase-9999_d.dll)
27/05/2014 21:02:53 (Error)   #23 SVGSample::initialise +0x47d (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISample_SVG_d.dll)
27/05/2014 21:02:53 (Error)   #24 SampleDataModule::initialise +0x65 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #25 SamplesFramework::initialiseSampleStepwise +0xb7 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #26 SamplesFramework::updateInitialisationStep +0x2a1 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #27 SamplesFramework::update +0x33 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #28 CEGuiBaseApplication::renderSingleFrame +0x6d (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #29 CEGuiGLFWSharedBase::drawFrame +0x65 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #30 CEGuiGLFWSharedBase::run +0x19f (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #31 CEGuiBaseApplication::execute +0x6e4 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #32 SamplesFrameworkBase::runApplication +0x2db (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #33 SamplesFrameworkBase::run +0x56 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #34 main +0xc0 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #35 __tmainCRTStartup +0x199 (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #36 mainCRTStartup +0xd (C:\Users\Luca\Documents\dev\cegui_tip_build\bin\CEGUISampleFramework-9999.0_d.exe)
27/05/2014 21:02:53 (Error)   #37 BaseThreadInitThunk +0xe (C:\WINDOWS\SYSTEM32\KERNEL32.DLL)
27/05/2014 21:02:53 (Error)   #38 RtlInitializeExceptionChain +0x84 (C:\WINDOWS\SYSTEM32\ntdll.dll)
27/05/2014 21:02:53 (Error)   #39 RtlInitializeExceptionChain +0x5a (C:\WINDOWS\SYSTEM32\ntdll.dll)
27/05/2014 21:02:53 (Error)   ==========  End of Backtrace  ==========
27/05/2014 21:02:53 (Error)   An unexpected error occurred while parsing XML
27/05/2014 21:02:53 (Error)   The last thrown exception was related to XML file 'SVGSampleImage.svg' from resource group 'imagesets'.
27/05/2014 21:02:53 (Error)   An unexpected error occurred while parsing XML
27/05/2014 21:02:53 (Error)   The last thrown exception was related to XML file 'SVGSampleImageset.imageset' from resource group 'imagesets'.
27/05/2014 21:02:55 (Std)    [ImageManager] Deleted image: cegui_logo
27/05/2014 21:02:55 (Std)    ---- Begining CEGUI System destruction ----

--snip--

27/05/2014 21:03:17 (Std)    ---- CEGUI System destruction completed ----
27/05/2014 21:03:17 (Std)    CEGUI::Logger singleton destroyed. (06FC85C0)


To fix this issue I developed the following patch (the important changes are the first and the last one, the others are only indentation adjustments):

Code: Select all

@@ -208,5 +208,8 @@
 
     void XercesParser::initialiseSchema(XERCES_CPP_NAMESPACE::SAX2XMLReader* reader, const String& schemaName)
     {
-        XERCES_CPP_NAMESPACE_USE;
+        // only load the schema if it's name is passed
+        if (!schemaName.empty())
+        {
+            XERCES_CPP_NAMESPACE_USE;
 
@@ -212,6 +215,6 @@
 
-        // enable schema use and set validation options
-        reader->setFeature(XMLUni::fgXercesSchema, true);
-        reader->setFeature(XMLUni::fgSAX2CoreValidation, true);
-        reader->setFeature(XMLUni::fgXercesValidationErrorAsFatal, true);
+            // enable schema use and set validation options
+            reader->setFeature(XMLUni::fgXercesSchema, true);
+            reader->setFeature(XMLUni::fgSAX2CoreValidation, true);
+            reader->setFeature(XMLUni::fgXercesValidationErrorAsFatal, true);
 
@@ -217,7 +220,7 @@
 
-        // load in the raw schema data
-        RawDataContainer rawSchemaData;
-        // load the schema from the resource group
-        Logger::getSingleton().logEvent("XercesParser::initialiseSchema - Attempting to load schema from file '" + schemaName + "'.");
-        System::getSingleton().getResourceProvider()->loadRawDataContainer(schemaName, rawSchemaData, d_defaultSchemaResourceGroup);
+            // load in the raw schema data
+            RawDataContainer rawSchemaData;
+            // load the schema from the resource group
+            Logger::getSingleton().logEvent("XercesParser::initialiseSchema - Attempting to load schema from file '" + schemaName + "'.");
+            System::getSingleton().getResourceProvider()->loadRawDataContainer(schemaName, rawSchemaData, d_defaultSchemaResourceGroup);
 
@@ -223,11 +226,11 @@
 
-        // wrap schema data in a xerces MemBufInputSource object
-        MemBufInputSource  schemaData(
-            rawSchemaData.getDataPtr(),
-            static_cast<const unsigned int>(rawSchemaData.getSize()),
-            schemaName.c_str(),
-            false);
-        reader->loadGrammar(schemaData, Grammar::SchemaGrammarType, true);
-        // enable grammar reuse
-        reader->setFeature(XMLUni::fgXercesUseCachedGrammarInParse, true);
+            // wrap schema data in a xerces MemBufInputSource object
+            MemBufInputSource  schemaData(
+                rawSchemaData.getDataPtr(),
+                static_cast<const unsigned int>(rawSchemaData.getSize()),
+                schemaName.c_str(),
+                false);
+            reader->loadGrammar(schemaData, Grammar::SchemaGrammarType, true);
+            // enable grammar reuse
+            reader->setFeature(XMLUni::fgXercesUseCachedGrammarInParse, true);
 
@@ -233,7 +236,7 @@
 
-        // set schema for usage
-        XMLCh* pval = XMLString::transcode(schemaName.c_str());
-        reader->setProperty(XMLUni::fgXercesSchemaExternalNoNameSpaceSchemaLocation, pval);
-        XMLString::release(&pval);
-        Logger::getSingleton().logEvent("XercesParser::initialiseSchema - XML schema file '" + schemaName + "' has been initialised.");
+            // set schema for usage
+            XMLCh* pval = XMLString::transcode(schemaName.c_str());
+            reader->setProperty(XMLUni::fgXercesSchemaExternalNoNameSpaceSchemaLocation, pval);
+            XMLString::release(&pval);
+            Logger::getSingleton().logEvent("XercesParser::initialiseSchema - XML schema file '" + schemaName + "' has been initialised.");
 
@@ -239,6 +242,12 @@
 
-        // use resource provider to release loaded schema data (if it supports this)
-        System::getSingleton().getResourceProvider()->unloadRawDataContainer(rawSchemaData);
+            // use resource provider to release loaded schema data (if it supports this)
+            System::getSingleton().getResourceProvider()->unloadRawDataContainer(rawSchemaData);
+        }
+        else
+        {
+            // otherwise ignore the missing schema and proceed
+            Logger::getSingleton().logEvent("XercesParser::initialiseSchema - No schema specified. Proceeding.");
+        }
     }
 
     XERCES_CPP_NAMESPACE::SAX2XMLReader* XercesParser::createReader(XERCES_CPP_NAMESPACE::DefaultHandler& handler)


After applying the patch everything worked fine.

In the current development state it seems that Xerces requires schemas to exist and does not handle the case they don't exist (I mean not to throw a critical exception). I wondered if it could be an improvement to let the "user" decide whether to throw an exception and shutdown the application, to show an error message and continue or to ignore it silently?

EDIT:
When I went to bed I spent some thoughts on my idea and I discovered that I made a not quite unimportant mistake: the error wasn't caused by a missing schema file but by a missing "schemaName" required by Xerces to load the schema (I think there was no "schemaName" because CEGUI has no svg schema shipped with). But the idea is still applicable: the "user" or programmer should be able to change the reporting behavior of Xerces' schema initialisation, because xml validation through schemas is no "built in" feature of CEGUI but a nice feature of Xerces (and Xerces is only one of five xml parsers that can be used with CEGUI and is the only one being capable of xml validation, so xml validation should also be an optional feature of the XercesParser module - maybe defaulting to "use it" but able to be switched off, for instance for svg loading where my error occurred).

[Ident]

If i get you correctly, your suggestion is that the xml validation in Xerces should be optional and default-on?

I principially agree with this idea. It would be best if CrazyEddie and Kulik also look at this topic before a decision is made. Also i am not sure how the validation should be best made optional - via cmake or via a function call.

Finally, i looked at your patch but i don't really know what is going on so I do not feel comfortable merging it. We prefer pull requests in bitbucket btw. But patches are alright too, just more difficult to handle for us.

[lucebac]

At first, thanks for reviewing it and please let CE and Kulik have a look on it (or make them have a look on it ).
Of course I will create a pull request if CE, Kulik and you agree to my idea and are willing to include this into CEGUI - the patch should only help to explain how i avoided the application shutdown due to a missing "schemaName". I will simplify the patch a little bit by removing the indentation adjustments as long as we discuss this.

And finally: Yes, you got my idea right - schema validation should be optional and default-on in Xerces.
In my opinion it would be best to globally disable schema validation via something like:

Code: Select all

CEGUI::XMLParser* parser = CEGUI::System::getSingleton().getXMLParser();
if (parser->isPropertyPresent("SchemaValidationEnabled"))
    parser->setProperty("SchemaValidationEnabled", "false");


And to locally disable validation by changing the prototype of the "parseXML" function(s) used by the xml parser modules (aiming especially on parsing svg files, because CEGUI does not have a schema for svg files):

Code: Select all

XMLParser.h (interface class and all implementations):
class XMLParser
{
//...
        void parseXML(XMLHandler& handler, const RawDataContainer& source, const String& schemaName, bool validate_xml = true);
//...
};

XMLParser.cpp (Xerces module):
void XercesParser::parseXML(XMLHandler& handler, const RawDataContainer& source, const String& schemaName, bool validate_xml /*= true*/)
{
//...
    CEGUI_TRY
    {
        // set up schema
        if (validate_xml)
            initialiseSchema(reader, schemaName);
        // do parse
        doParse(reader, source);
    }
// catch errors and so on...


[Ident]

Quote from #cegui after you had left:

[17:06:24] mpreisler this looks like a very nice patch
[17:06:26] mpreisler for default

(default branch is meant of course)

[lucebac]

Thanks for posting. Maybe i should come back to the irc channel.
I will create a pull request and I hope CE won't have any reasons to decline it afterwards...

[Ident]

That would be great thanks. And btw sorry for my late response, i was very busy with university so I didnt go to the forums at all. Usually i check by the forums every 1-2 days at least.

[lucebac]

No problem, i am a student too, i know how time consuming it can be.

[Ident]

I wrote a mail to CrazyEddie, he replied:

I have no objections to this; seems like a good idea. Might even have been mentioned in passing before, but never put into action.

[lucebac]

Thanks, Ident.
I will start working on it when i have finished the implementation of the SplitButton (it was mentioned somewhere on the board months (or even years) ago and was digged out last month or in april). I'm still struggling with the internal event handler(s) and handling mechanisms...but I will have a look into the docu once more - maybe it helps; if not you'll see it on the board

[Ident]

Cool, glad to hear about that.