Review Request 118310: support for graphic properties of Geogebra objects in KIG

David Narváez david.narvaez at computer.org
Thu Nov 27 02:57:04 UTC 2014



> On Nov. 18, 2014, 11:51 a.m., David Narváez wrote:
> > geogebra/geogebratransformer.cpp, line 154
> > <https://git.reviewboard.kde.org/r/118310/diff/2/?file=328744#file328744line154>
> >
> >     Use int ObjectDrawer::pointStyleFromString( const QString& style ), which is evil but not worse than magic constants.
> >     
> >     I'm thinking this pointType thing should be an Enum and we could use the Q_ENUM facility instead of these conversions, but this is out of the scope of this patch.
> 
> Aniket Anvit wrote:
>     I saw the documentation on how to use Q_ENUM along with Q_PROPERTY and I liked it. Tell me if you would like to go with it.

Use int ObjectDrawer::pointStyleFromString( const QString& style ) for now


> On Nov. 18, 2014, 11:51 a.m., David Narváez wrote:
> > geogebra/geogebratransformer.cpp, line 158
> > <https://git.reviewboard.kde.org/r/118310/diff/2/?file=328744#file328744line158>
> >
> >     Set it to RectangularEmpty and then either figure out why is it not drawing, or add a TODO comment here

I can't guess what not drawing fine means in this context, but if Kig doesn't crash, use int ObjectDrawer::pointStyleFromString( const QString& style ) regardless, don't just set it to 0. If it doesn't work we'll have to figure out why later.


> On Nov. 18, 2014, 11:51 a.m., David Narváez wrote:
> > geogebra/geogebratransformer.cpp, line 156
> > <https://git.reviewboard.kde.org/r/118310/diff/2/?file=328744#file328744line156>
> >
> >     Set it to RoundEmpty and then either figure out why is it not drawing, or add a TODO comment here
> 
> Aniket Anvit wrote:
>     All the pointStyles are drawing fine except the hollow ones ( RoundEmpty and RectangularEmpty ). Can't seem to figure out why!

I can't guess what not drawing fine means in this context, but if Kig doesn't crash, use int ObjectDrawer::pointStyleFromString( const QString& style ) regardless, don't just set it to 0. If it doesn't work we'll have to figure out why later.


> On Nov. 18, 2014, 11:51 a.m., David Narváez wrote:
> > geogebra/geogebrasection.h, line 55
> > <https://git.reviewboard.kde.org/r/118310/diff/2/?file=328741#file328741line55>
> >
> >     If there will be exactly one drawer in m_drawers as there will be output objects in m_outputObjects, make m_outputObjects a vector of pair of ObjectCalcer* and ObjectDrawer*
> 
> Aniket Anvit wrote:
>     Actually I had thought of it but we can't use a vector pair since while reading tools we would need to construct objectHierarchy from the inputObjects and the outputObjects. If the outputObjects is a vector< pair<ObjectCalcer*, ObjectDrawer* >> then it won't be possible.

Right, nice catch. Add a comment explaining m_drawers needs to be kept separate for that reason.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118310/#review70577
-----------------------------------------------------------


On Nov. 21, 2014, 1:43 p.m., Aniket Anvit wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118310/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2014, 1:43 p.m.)
> 
> 
> Review request for KDE Edu and David Narváez.
> 
> 
> Repository: kig
> 
> 
> Description
> -------
> 
> This patch enables KIG to read the graphic properties of Geogebra objects from Geogebra worksheets.
> Here is some explanation about my implementation:
>   
> the graphic properties ( lineStyle, pointStyle, lineWidth, color ) are read for each geogebra object. I add an attribute for each property to each of the intermediate representation of each of the objects created using XSLT. Some variables are added to the KigFilterGeogebra class to hold the values of the properties for each of the objects. In the attribute() callback method of the KigFilterGeogebra class , the values of the properties are read ( and suitably mapped to KIG's ) and when the endElement() callback is called a new ObjectDrawer with the last read properties is allocated for this object.
> 
> 
> Diffs
> -----
> 
>   filters/geogebra-filter.cpp cff342d 
>   geogebra/geogebra.xsl c1e4749 
>   geogebra/geogebrasection.h aed7dcb 
>   geogebra/geogebrasection.cpp e9778ed 
>   geogebra/geogebratransformer.h 5f36827 
>   geogebra/geogebratransformer.cpp aee8669 
> 
> Diff: https://git.reviewboard.kde.org/r/118310/diff/
> 
> 
> Testing
> -------
> 
> Tested! some trouble in the mapping of PointStyles.
> 
> 
> File Attachments
> ----------------
> 
> Test file
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/11/15/9799362b-472e-4814-9d78-da2bf07d612c__gui.ggb
> 
> 
> Thanks,
> 
> Aniket Anvit
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20141127/e2f81356/attachment-0001.html>


More information about the kde-edu mailing list