View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0024181 | Open CASCADE | OCCT:Visualization | public | 2013-09-16 19:04 | 2019-09-17 11:11 |
Reporter | Assigned To | bugmaster | |||
Priority | normal | Severity | feature | ||
Status | closed | Resolution | fixed | ||
Platform | A | OS | L | ||
Product Version | Unscheduled | ||||
Target Version | 6.7.0 | Fixed in Version | 6.7.0 | ||
Summary | 0024181: Text to BRep functionality | ||||
Description | There are many cases when text should be a part of CAD model, not just auxiliary overlay labels in 3D viewer. However currently OCCT doesn't provide built-in way to generate such shapes. This feature is intended to introduce such functionality basing on FreeType library which is already used by TKOpenGl text rendering. | ||||
Tags | No tags attached. | ||||
Test case number | 3rdparty fonts(002) B1, B2, B3 | ||||
related to | 0024218 | closed | bugmaster | Open CASCADE | ShapeFix_Face requires double execution to produce valid shape when FixSplitFaceMode is in effect |
parent of | 0030911 | closed | Open CASCADE | Visualization - Font_BRepFont using as a usual Standard_Transient | |
parent of | 0030971 | new | Community | Visualization - can Font_BRepFont generate composite curve to C1 rather than to C0? | |
related to | 0024220 | closed | bugmaster | Open CASCADE | bopargcheck returns valid for C0 shape but results of boolean operations are broken with such shapes |
related to | 0024289 | closed | Open CASCADE | Linkage issue on TKViewerTest when built from cbp projects | |
related to | 0025852 | closed | bugmaster | Open CASCADE | Visualization - Font_BRepFont produces bad faces for circled symbols |
Not all the children of this issue are yet resolved or closed. |
|
please review branch cr24181 |
|
Dear isn, please implement new Draw Harness commands as well as test case for new functionality (and please attach demonstrative screenshots to this bug). +TopoDS_Shape Font_TextToBRep::RenderGlyph (const unsigned int theChar, + NCollection_DataMap<unsigned int, TopoDS_Shape> myCache; //!< cached (already computed) glyphs Please use adopted type aliases for Unicode symbols (Standard_Utf32Char). >> lin,p1,p2,my3Poles,my4Poles Please follow OCCT name conventions. + BRep_Builder aBuilder; + TColgp_Array1OfPnt2d my3Poles (1, 3); Please consider caching fat temporary variables as algorithm class field. This algorithm can not be used from parallel threads nevertheless (which should be documented by the way) due to current FreeType logic. + if (aWireMaker.IsDone()) + { + TopoDS_Wire aWire = aWireMaker; + //! @return true on success + Standard_EXPORT virtual bool Init (const NCollection_String& theFontPath, + const unsigned int thePointSize, + const unsigned int theResolution = 72); nit: broken indentation. +TopoDS_Shape Font_TextToBRep::RenderGlyph (const TCollection_AsciiString& theString, Method name doesn't correspond to it arguments. Also current implementation could be applied only to ANSI symbols. Please provide Unicode implementation as well in similar way as OpenGl_TextFormatter does (using NCollection_String argument and appropriate iterator). + + Standard_EXPORT Font_TextToBRep(); + + Standard_EXPORT ~Font_TextToBRep(); Every public method should be documented (even if description is trivial). Overridden method should be marked with virtual keyword. + static inline gp_XY ReadFTVec (const FT_Vector& theVec) It is preferred to call private methods lowercase. |
|
branch cr24181 was updated, please check. new draw command was added |
2013-09-18 20:40 developer |
screen.GIF (5,431 bytes) |
|
Dear isn, please look again to my previous remarks list - not all of them have been fixed. New remarks following. +TopoDS_Shape Font_TextToBRep::RenderGlyph (const Standard_Utf32Char theChar, + const gp_XYZ thePen) Such structures (gp_XYZ) should be passed as cost reference. +Font_TextToBRep::Font_TextToBRep() +: myCache(), + my3Poles (1, 3), + my4Poles (1, 4), + Font_FTFont() Broken initialization order. Anyway fields with default constructors might be skipped at all. + myMutex.Lock(); + const gp_Pln aPln(gp::XOY()); + gp_Trsf aTrsf; + aTrsf.SetTranslation (*(gp_Vec* )&thePen); + + if (!loadGlyph (theChar) + || myFTFace->glyph->format != FT_GLYPH_FORMAT_OUTLINE) + { + return TopoDS_Shape(); + } + else if (myCache.IsBound (theChar)) + { + return myCache.Find (theChar).Moved (aTrsf); + } Mutex is not released! Please consider usage of Standard_Mutex::Sentry. + aConcatMaker.Add(aBoundedCurves.Value(i),1e-5); Nameless constants should be avoided. +bool Font_TextToBRep::Init (const NCollection_String& theFontPath, + const unsigned int thePointSize, + const unsigned int theResolution) +{ + myCache.Clear(); It makes sense to make Release method virtual and clean cache there instead. + Font_TextToBRep T; + T.Init(argv[3], atoi(argv[4])); Please follow name conventions. Result of algorithm initialization should be checked in test command. |
|
branch was updated |
|
Remarks from abv: 1. Please drop redundant destructor. 2. In class description: "render text as BRep" - ? maybe "convert text to BRep"? 3. Please eliminate this pointers math (redundant performance hack): >> aTrsf.SetTranslation (*(gp_Vec* )&thePen); 4. Please use Sentry for Mutex 5. Maybe >> (aPntsNb - 1) % aPntsNb should be just >> (aPntsNb - 1) ? 6. It will be better to create TopoDS_Wire composed from multiple TopoDS_Edge's rather than single TopoDS_Edge with composite curve 7. It will be better to create TopoDS_Face rather than TopoDS_Compound of wires |
|
remarks 1-5 (from abv) was processed, branch was updated |
|
One more remark: please provide test case for this functionality, and consider writing message for dev portal announcing appearance of this new feature, with some nice pictures. |
|
Font_FontMgr (instead of fontpath) added, remark 6 was processed. branch was updated |
|
+bool Font_TextToBRep::Init (const NCollection_String& theFontName, + const unsigned int thePointSize, + const unsigned int theResolution) +{ + Handle(Font_FontMgr) aFontMgr = Font_FontMgr::GetInstance(); + const Handle(TCollection_HAsciiString) aFontName = new TCollection_HAsciiString (theFontName.ToCString()); + Handle(Font_SystemFont) aRequestedFont = aFontMgr->FindFont (aFontName, Font_FA_Regular, thePointSize); This method should take full font path by current design (in base class). I consider to add new method to the base class with additional Font_FontAspect argument which doing what current patch propose. +TopoDS_Shape Font_TextToBRep::Render (const Standard_Utf32Char theChar, + bool IsOneCurve, + const gp_XYZ& thePen) New option "IsOneCurve" should be class field - not public method argument. + if (IsOneCurve) + { + Geom2dConvert_CompCurveToBSplineCurve aConcatMaker(aBoundedCurves.Value(1)); + for (int i = 2; i <= aBoundedCurves.Length(); i++) + aConcatMaker.Add(aBoundedCurves.Value(i), Precision::Confusion()); + Handle_Geom2d_BSplineCurve aRes = aConcatMaker.BSplineCurve(); + BRepBuilderAPI_MakeEdge anEdgeBuilder(GeomAPI::To3d(aRes, aPln)); + aWireMaker.Add (anEdgeBuilder.Edge()); I propose to drop extra temporary collection of curves in "aBoundedCurves". Geom2dConvert_CompCurveToBSplineCurve might be improved for this purpose if needed. |
|
branch was updated, some test cases was added. |
|
branch was updated again |
|
textOnBottle.png (443,331 bytes) |
|
Patch is ready for review in CR24181_2 branch (history in CT24181_1). |
|
No remarks, please test |
|
Dear BugMaster, Branch CR24181_2 (and products from GIT master) was compiled on Linux and Windows platforms. There are compilation errors on Linux: http://jenkins-test-02.nnov.opencascade.com:8080/user/mnt/my-views/view/CR24181_2/job/mnt-CR24181_2-master_build_occt_linux/2/parsed_console/? In file included from ../../../../inc/Font_FTLibrary.hxx:28, from ../../../../inc/Font_FTFont.hxx:25, from ../../../../inc/Font_BRepFont.hxx:24, from ../../../../src/ViewerTest/ViewerTest_ObjectCommands.cxx:35: /usr/include/ft2build.h:56:38: error: freetype/config/ftheader.h: No such file or directory |
|
Compilation issue on WOK should be fixed now. In advance verase now prints the list of erased objects. |
|
Is it possible to add one more test case - to test how sample text looks if triangulated with different deflection values? tests/3rdparty/fonts/B3 can be used as the starting point. And probably vfps command should be called for each deflection value to see how deflection affects performance. |
|
Dear dbv, please create scripts and estimate performance as suggested by san in scope of your current activity #0024133 |
|
Branch CR24181_2 reviewed without remarks, ready for testing. |
|
Patch has been updated to fix issues with generated automake scripts |
|
Dear BugMaster, Branch CR24181_2 (and products from GIT master) was compiled on Linux and Windows platforms and tested. SHA-1: 417af4eab144b767173af8d3825a77099fa6a930 Number of compiler warnings: occt component : Linux: 423 (424 on master) Windows: 9 (9 on master) products component : Linux: (189 on master) Windows: 287 (287 on master) Regressions/Differences: No regressions/differences Testing cases: http://occt-tests/CR24181-22-master-occt/Debian60-64/3rdparty/fonts/B1.html http://occt-tests/CR24181-22-master-occt/Windows-32-VC9/3rdparty/fonts/B1.html 3rdparty fonts(002) B1: OK http://occt-tests/CR24181-22-master-occt/Debian60-64/3rdparty/fonts/B2.html http://occt-tests/CR24181-22-master-occt/Windows-32-VC9/3rdparty/fonts/B2.html 3rdparty fonts(002) B2: OK http://occt-tests/CR24181-22-master-occt/Debian60-64/3rdparty/fonts/B3.html http://occt-tests/CR24181-22-master-occt/Windows-32-VC9/3rdparty/fonts/B3.html 3rdparty fonts(002) B3: OK Testing on Linux: Total MEMORY difference: 351582356 / 366893108 Total CPU difference: 41158.96000000109 / 44590.37000000126 Testing on Windows: Total MEMORY difference: 402171980 / 433338864 Total CPU difference: 46183.0 / 41201.890625 There are following differences in images found by testdiff. http://occt-tests/CR24181-22-master-occt/Debian60-64/diff-Debian60-64.html http://occt-tests/CR24181-22-master-occt/Windows-32-VC9/diff-Windows-32-VC9.html IMAGE demo samples bottle: bottle.png differs |
occt: master b514beda 2013-10-10 09:35:04
Committer: bugmaster Details Diff |
0024181: Text to BRep functionality Introduce new class Font_BRepFont for conversion of font glyph in vector format into BRep representation. New text2brep Draw Harness command. bottle.tcl - draw text on the bottle side using new functionality. ViewerTest - process Delete key in 3D-Viewer to delete selected presentations. Font_FontMgr::FindFont - return correct font when font alias and not default aspect is requested. bottle.tcl - use prism instead of pipe TKViewerTest - add required FreeType dependency verase - display the list of erase objects TKViewerTest - add required FreeType dependency for projects generation |
Affected Issues 0024181 |
|
mod - samples/tcl/bottle.tcl | Diff File | ||
mod - src/Font/FILES | Diff File | ||
mod - src/Font/Font.cdl | Diff File | ||
add - src/Font/Font_BRepFont.cxx | Diff File | ||
add - src/Font/Font_BRepFont.hxx | Diff File | ||
mod - src/Font/Font_FontMgr.cxx | Diff File | ||
mod - src/Font/Font_FTFont.cxx | Diff File | ||
mod - src/Font/Font_FTFont.hxx | Diff File | ||
mod - src/Geom2dConvert/Geom2dConvert_CompCurveToBSplineCurve.cdl | Diff File | ||
mod - src/Geom2dConvert/Geom2dConvert_CompCurveToBSplineCurve.cxx | Diff File | ||
mod - src/GeomConvert/GeomConvert_CompCurveToBSplineCurve.cdl | Diff File | ||
mod - src/GeomConvert/GeomConvert_CompCurveToBSplineCurve.cxx | Diff File | ||
mod - src/TKService/EXTERNLIB | Diff File | ||
mod - src/TKViewerTest/EXTERNLIB | Diff File | ||
mod - src/ViewerTest/EXTERNLIB | Diff File | ||
mod - src/ViewerTest/ViewerTest.cxx | Diff File | ||
mod - src/ViewerTest/ViewerTest_ObjectCommands.cxx | Diff File | ||
mod - src/ViewerTest/ViewerTest_ViewerCommands.cxx | Diff File | ||
mod - tests/3rdparty/begin | Diff File | ||
add - tests/3rdparty/fonts/B1 | Diff File | ||
add - tests/3rdparty/fonts/B2 | Diff File | ||
add - tests/3rdparty/fonts/B3 | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-09-16 19:04 |
|
New Issue | |
2013-09-16 19:04 |
|
Assigned To | => isn |
2013-09-17 08:32 | kgv | Severity | minor => feature |
2013-09-17 08:32 | kgv | Reproducibility | have not tried => N/A |
2013-09-17 08:32 | kgv | Status | new => assigned |
2013-09-17 08:32 | kgv | Target Version | Unscheduled => 6.7.0 |
2013-09-17 08:32 | kgv | Summary | TextToBrep functionality => Text to BRep functionality |
2013-09-17 08:32 | kgv | Description Updated | |
2013-09-17 17:59 |
|
Status | assigned => resolved |
2013-09-17 18:00 |
|
Assigned To | isn => kgv |
2013-09-17 18:00 |
|
Note Added: 0025606 | |
2013-09-17 22:12 | kgv | Note Added: 0025609 | |
2013-09-17 22:12 | kgv | Assigned To | kgv => isn |
2013-09-17 22:12 | kgv | Status | resolved => assigned |
2013-09-17 22:14 | kgv | Note Edited: 0025609 | |
2013-09-17 22:17 | kgv | Note Edited: 0025609 | |
2013-09-17 22:18 | kgv | Note Edited: 0025609 | |
2013-09-18 20:39 |
|
Note Added: 0025637 | |
2013-09-18 20:40 |
|
File Added: screen.GIF | |
2013-09-18 20:41 |
|
Status | assigned => resolved |
2013-09-18 20:41 |
|
Assigned To | isn => kgv |
2013-09-18 20:42 |
|
Note Edited: 0025637 | |
2013-09-18 21:03 | kgv | Note Added: 0025638 | |
2013-09-18 21:03 | kgv | Assigned To | kgv => isn |
2013-09-18 21:03 | kgv | Status | resolved => assigned |
2013-09-19 15:55 |
|
Note Added: 0025656 | |
2013-09-19 15:55 |
|
Assigned To | isn => kgv |
2013-09-19 15:55 |
|
Status | assigned => resolved |
2013-09-19 17:53 | kgv | Note Added: 0025663 | |
2013-09-19 17:53 | kgv | Assigned To | kgv => isn |
2013-09-19 17:53 | kgv | Status | resolved => assigned |
2013-09-19 18:27 |
|
Note Added: 0025664 | |
2013-09-20 06:31 |
|
Note Added: 0025665 | |
2013-09-20 17:51 |
|
Note Added: 0025684 | |
2013-09-20 17:55 |
|
Note Edited: 0025684 | |
2013-09-20 18:20 | kgv | Note Added: 0025685 | |
2013-09-24 19:06 |
|
Note Added: 0025718 | |
2013-09-24 19:06 |
|
Assigned To | isn => kgv |
2013-09-25 11:50 | kgv | Assigned To | kgv => isn |
2013-09-26 19:06 |
|
Note Added: 0025757 | |
2013-10-01 11:53 | kgv | File Added: textOnBottle.png | |
2013-10-01 16:48 | kgv | Note Added: 0025827 | |
2013-10-01 16:48 | kgv | Assigned To | isn => abv |
2013-10-01 16:48 | kgv | Status | assigned => resolved |
2013-10-01 16:50 | kgv | Relationship added | related to 0024218 |
2013-10-02 10:09 | kgv | Relationship added | related to 0024220 |
2013-10-02 15:42 |
|
Note Added: 0025848 | |
2013-10-02 15:42 |
|
Assigned To | abv => bugmaster |
2013-10-02 15:42 |
|
Status | resolved => reviewed |
2013-10-02 15:55 |
|
Assigned To | bugmaster => mkv |
2013-10-03 10:37 |
|
Note Added: 0025856 | |
2013-10-03 10:37 |
|
Assigned To | mkv => kgv |
2013-10-03 10:37 |
|
Status | reviewed => assigned |
2013-10-03 14:08 | kgv | Note Added: 0025870 | |
2013-10-03 14:08 | kgv | Assigned To | kgv => abv |
2013-10-03 14:08 | kgv | Status | assigned => resolved |
2013-10-03 20:12 |
|
Note Added: 0025880 | |
2013-10-03 20:12 |
|
Assigned To | abv => kgv |
2013-10-03 20:12 |
|
Status | resolved => assigned |
2013-10-03 20:13 |
|
Note Edited: 0025880 | |
2013-10-04 08:03 | kgv | Assigned To | kgv => dbv |
2013-10-04 08:05 | kgv | Note Added: 0025882 | |
2013-10-04 17:02 |
|
Status | assigned => resolved |
2013-10-04 17:03 |
|
Note Added: 0025906 | |
2013-10-04 17:03 |
|
Assigned To | dbv => bugmaster |
2013-10-04 17:03 |
|
Status | resolved => reviewed |
2013-10-07 08:32 |
|
Assigned To | bugmaster => mkv |
2013-10-07 10:34 | kgv | Note Added: 0025927 | |
2013-10-08 12:32 |
|
Note Added: 0025954 | |
2013-10-08 12:33 |
|
Note Edited: 0025954 | |
2013-10-08 12:33 |
|
Test case number | => 3rdparty fonts(002) B1, B2, B3 |
2013-10-08 12:33 |
|
Assigned To | mkv => bugmaster |
2013-10-08 12:33 |
|
Status | reviewed => tested |
2013-10-11 13:35 | bugmaster | Changeset attached | => occt master b514beda |
2013-10-11 13:35 | bugmaster | Status | tested => verified |
2013-10-11 13:35 | bugmaster | Resolution | open => fixed |
2013-10-25 09:46 | kgv | Relationship added | related to 0024289 |
2013-12-19 13:51 | bugmaster | Status | verified => closed |
2013-12-19 13:58 | bugmaster | Fixed in Version | => 6.7.0 |
2015-02-23 16:16 |
|
Relationship added | related to 0025852 |
2019-09-04 16:19 | kgv | Relationship added | parent of 0030911 |
2019-09-17 11:11 | kgv | Relationship added | parent of 0030971 |