taskGnash - The GNU Flash player - Tasks: task #5837, Remove rendering calls from parser

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #5837: Remove rendering calls from parser

Submitter:  Sandro Santilli <strk>
Submitted:  Tue 22 Aug 2006 08:13:14 PM UTC
   
 
Should Start On:  Mon 21 Aug 2006 10:00:00 PM UTC Should be Finished on:  Tue 12 Sep 2006 10:00:00 PM UTC
Category:  None Priority:  7 - High
Status:  Cancelled Privacy:  Public
Assigned to:  nihilus Percent Complete:  100%
Open/Closed:  Closed Effort:  0.00

Jump to the original submission

Fri 15 Oct 2010 09:11:18 AM UTC, comment #20: 

The renderer is still used during parsing for storing cached bitmaps, but this is a reasonable design decision.

Benjamin Wolsey <bwy>
Group Member
Thu 18 Sep 2008 07:25:44 AM UTC, comment #19: 

Oops, sorry, wrong bug :/

Sandro Santilli <strk>
Group Member
Thu 18 Sep 2008 07:24:31 AM UTC, comment #18: 


              |                     matrix caches                 |    
              |                                                   |
              |           on            |           off           |
 -------------+-------------------------+-------------------------+
   d          | - mario walks backward  | - mario walks backward  |    
   e    use   | - snowy OK              | - snowy shots backward  |    
   t          |                         |                         |
   e   -------+-------------------------+-------------------------+
   r          | - mario walks backward  | - mario OK              |
   m    no    | - snowy walks backward  | - snowy walks backward  |
              |                         |                         |
 -------------+-------------------------+-------------------------+

Sandro Santilli <strk>
Group Member
Mon 25 Sep 2006 03:24:28 PM UTC, comment #17: 

I split the bitmap_character class into _def and _instance.
You can now create a _def w/out having a bitmap_info, still the bitmap_info will be created lazily on get_bitmap_info() call.
Unfortunately, this is not enough, as get_bitmap_info() seems to be still invoked during parsing.

One place is in movie_def_impl::add_bitmap_character_def.
Other places are inside style.cpp.

Triggered an abort on cases in which a bitmap_info is attempted
to be constructed but a renderer is not been registered. This will hopefully help us tracking down the different calls to it.

Sandro Santilli <strk>
Group Member
Mon 25 Sep 2006 01:50:48 PM UTC, comment #16: 

It seems that part of the problem is the bitmap_character class.
Differently from other classes the bitmap_character_def inherits from character_def and bitmap_character inherits from bitmap_character_def !!

This needs to be cleaned up.

The only class that the parser should create is the bitmap_character_def, which shoudl not create a bitmap info.

The bitmap_info (which seems a cache to me) should be created by the instance (bitmap_character, that needs to derive from character) instead and, if we want, added to the _def for fast access from multiple instances.

Sandro Santilli <strk>
Group Member
Mon 25 Sep 2006 01:42:23 PM UTC, comment #15: 

Not specifically, although the same problem occurs with caching.

Bastiaan Jacques <bjacques>
Group Member
Mon 25 Sep 2006 01:26:51 PM UTC, comment #14: 

Are we talking about the cache here ?
Asking because we were discussing about cache with Udo as well

Sandro Santilli <strk>
Group Member
Mon 25 Sep 2006 01:17:16 PM UTC, comment #13: 

I suspect it's possible to delay creating the bitmap_info structures until it's time to actually render the corresponding objects.

Bastiaan Jacques <bjacques>
Group Member
Mon 25 Sep 2006 01:08:31 PM UTC, comment #12: 

Would it be possible to decouple bitmap_info objects from any renderer (make it indipendent from renrerer?)

Sandro Santilli <strk>
Group Member
Mon 25 Sep 2006 12:05:17 PM UTC, comment #11: 

Removing rid of create_bitmap_info_xyz() calls is a prerequisite for Cairo rendering to work at all. The main problem is that read_all_swf() and friends are called before a renderer is created. This means that the renderer will later be fed bogus bitmap_info structures. This in turn crashes the Cairo renderer, and I think it'll produce invalid output for other renderers.

Bastiaan Jacques <bjacques>
Group Member
Sat 26 Aug 2006 10:26:09 PM UTC, comment #10: 

Yes, probably. See also stuff in tag_loaders (grep for render::).
I think attaching the bitmaps to definition was a choice of caching, but the implementation doesn't look nice anyway...

Sandro Santilli <strk>
Group Member
Sat 26 Aug 2006 09:23:34 PM UTC, comment #9: 

I suppose you mean create_bitmap_INFO_rgb{a}...

Markus Gothe <nihilus>
Group Member
Sat 26 Aug 2006 11:48:18 AM UTC, comment #8: 

Yeah, I can do that... I just didn't wanted to change to much of the code... But it will definitly speed up loading time, which is a good thing.

Markus Gothe <nihilus>
Group Member
Sat 26 Aug 2006 08:51:21 AM UTC, comment #7: 

I confirm I can't reproduce "shock absorber adv." (jumping boobs)
text bug anymore (altought fill_style still calls render::create_bitmap_rbg{a}).

Still think it's worth moving 'render' namespace definitions
in a separate header and making sure none of parser code
includes or uses those funx.

Sandro Santilli <strk>
Group Member
Sat 26 Aug 2006 08:40:30 AM UTC, comment #6: 

I got reports about your changes fixing some existing problems.
Anyway, I was tracking randering calls and found that fill_style::read (a "parser" method in styles.cpp) still calls the renderer (create_bitmap_rgb and create_bitmap_rgba).

I think we should remove those calls as well, for complete isolation.

For testing purposes, what I did was having movie_def_impl::read
directly read_all_swf and finally exit, thus avoiding any
movie advance, and have all of render:: functions print a message.

Sandro Santilli <strk>
Group Member
Thu 24 Aug 2006 09:34:39 PM UTC, comment #5: 

Indeed, I backtracked the process. Seem like glEnable() is one of the culprits here...

Markus Gothe <nihilus>
Group Member
Thu 24 Aug 2006 12:03:22 AM UTC, comment #4: 

I don't believe NPTL would make any difference. The problem is that if more than one thread access the OpenGL or X11 display seperately, they can conflict. This conflict is timing based, so it works on some systems and not on other. The trick is to properly get the X11 and OpenGL contexts, and then swap them. In this case the parsing thread shouldn't access the graphics context anyway. The original pluging actually worked this way.


Rob Savoye <rsavoye>
Group administrator
Wed 23 Aug 2006 09:03:58 PM UTC, comment #3: 

I don't know what NPTL is, anyway I can't reproduce any segfault
with my setup:

- linux 2.6.7
- libc6          2.3.2.ds1-22


Sandro Santilli <strk>
Group Member
Wed 23 Aug 2006 07:07:26 PM UTC, comment #2: 

I think this might only affect systems based on only NPTL based systems... Can anyone running Linux Threads and glibc < 2.4 confirm by testing and then switching to NPTL and try it?

Markus Gothe <nihilus>
Group Member
Tue 22 Aug 2006 08:24:23 PM UTC, comment #1: 

To verify a problem is related to this "bug" you can try building
the code with LOAD_MOVIES_IN_A_SEPARATE_THREAD undefined in server/movie_def_impl.cpp.

Sandro Santilli <strk>
Group Member
Tue 22 Aug 2006 08:13:14 PM UTC, original submission:  

Some parsing methods (read funx of some definition classes) currently call rendering API, which (at least for OpenGL) seems to have problems working fine with threads.

Some bugs that seems related are: #17434, #17495

intro.swf (the "boobs shok absorber advertisement") seems also
to be suffering from this problem, which is intermittent: fonts
are sometime rendered fine, sometime not, possibly still related
to create_bitmap_rgba and friends calls (suppose it's glyphs rendering).

Sandro Santilli <strk>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bwy (Posted a comment)
  • -email is unavailable- added by strk (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 18 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-10-15 bwy StatusIn Progress Cancelled
        Open/ClosedOpen Closed
    2006-09-25 strk StatusReady For Test In Progress
    2006-09-06 nihilus Should be Finished on2006-09-06 2006-09-12
    2006-08-26 strk StatusDone Ready For Test
        Open/ClosedClosed Open
    2006-08-24 nihilus Percent Complete70% 100%
        Open/ClosedOpen Closed
    2006-08-24 nihilus Percent Complete100% 70%
        Open/ClosedClosed Open
    2006-08-24 nihilus StatusIn Progress Done
        Percent Complete30% 100%
        Open/ClosedOpen Closed
    2006-08-24 nihilus Percent Complete0% 30%
    2006-08-24 nihilus Should be Finished on2006-08-21 2006-09-06
        StatusNone In Progress
    2006-08-23 nihilus Assigned toNone nihilus
    2006-08-22 nihilus Carbon-Copy- Added nihilus

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code