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 authentification level.

task #5837: Remove rendering calls from parser

Submitted by:  Sandro Santilli <strk>
Submitted on:  Tue 22 Aug 2006 08:13:14 PM UTC  
 
Should Start On: Mon 21 Aug 2006 10:00:00 PM UTCShould be Finished on: Tue 12 Sep 2006 10:00:00 PM UTC
Category: NonePriority: 7 - High
Status: CancelledPrivacy: Public
Percent Complete: 100%Assigned to: Markus Gothe <nihilus>
Open/Closed: ClosedEffort: 0.00

(Jump to the original submission 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>
Project Member
Thu 18 Sep 2008 07:25:44 AM UTC, comment #19:

Oops, sorry, wrong bug :/

Sandro Santilli <strk>
Project 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>
Project 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>
Project 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>
Project Member
Mon 25 Sep 2006 01:42:23 PM UTC, comment #15:

Not specifically, although the same problem occurs with caching.

Bastiaan Jacques <bjacques>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project Member
Sat 26 Aug 2006 09:23:34 PM UTC, comment #9:

I suppose you mean create_bitmap_INFO_rgb{a}...

Markus Gothe <nihilus>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project 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>
Project 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>
Project MemberIn charge of this item.
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>
Project 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>
Project 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>
Project MemberIn charge of this item.
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>
Project 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>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 18 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 15 Oct 2010 09:11:18 AM UTCbwyStatusIn Progress=>Cancelled
      Open/ClosedOpen=>Closed
    Mon 25 Sep 2006 01:26:51 PM UTCstrkStatusReady For Test=>In Progress
    Wed 06 Sep 2006 05:25:53 PM UTCnihilusShould be Finished onWed 06 Sep 2006 10:00:00 PM UTC=>Tue 12 Sep 2006 10:00:00 PM UTC
    Sat 26 Aug 2006 08:40:30 AM UTCstrkStatusDone=>Ready For Test
      Open/ClosedClosed=>Open
    Thu 24 Aug 2006 11:23:14 PM UTCnihilusPercent Complete70%=>100%
      Open/ClosedOpen=>Closed
    Thu 24 Aug 2006 11:18:46 PM UTCnihilusPercent Complete100%=>70%
      Open/ClosedClosed=>Open
    Thu 24 Aug 2006 10:57:05 PM UTCnihilusStatusIn Progress=>Done
      Percent Complete30%=>100%
      Open/ClosedOpen=>Closed
    Thu 24 Aug 2006 10:36:12 PM UTCnihilusPercent Complete0%=>30%
    Thu 24 Aug 2006 07:49:41 PM UTCnihilusShould be Finished onMon 21 Aug 2006 10:00:00 PM UTC=>Wed 06 Sep 2006 10:00:00 PM UTC
      StatusNone=>In Progress
    Wed 23 Aug 2006 04:48:54 PM UTCnihilusAssigned toNone=>nihilus
    Tue 22 Aug 2006 09:27:38 PM UTCnihilusCarbon-Copy-=>Added nihilus

    Back to the top


    Powered by Savane 3.1-cleanup