ScaleBrush doesn't work for brushes from PDF

Discuss PDF file handling with the Polybios plugin here
Post Reply
User avatar
jPV
Posts: 603
Joined: Sat Mar 26, 2016 10:44 am
Location: RNO
Contact:

ScaleBrush doesn't work for brushes from PDF

Post by jPV »

Code: Select all

@REQUIRE "polybios"
Repeat
f$=FileRequest("Select a PDF file")
If Not f$ Then End
pdf.OpenDocument(1, f$)
pdf.GetBrush(1, 1, 1)
DebugPrint("Orig dimensions:", GetAttribute(#BRUSH,1,#ATTRWIDTH), GetAttribute(#BRUSH,1,#ATTRHEIGHT))
ScaleBrush(1,"200%",#KEEPASPRAT) ; <- Any kind of scaling results an 1x1 brush always
DebugPrint("New dimensions:", GetAttribute(#BRUSH,1,#ATTRWIDTH), GetAttribute(#BRUSH,1,#ATTRHEIGHT))
DisplayBrush(1, #CENTER, #CENTER) ; <- This doesn't show anything then
FreeBrush(1)
pdf.CloseDocument(1)
WaitLeftMouse
Forever
The output from the previous script is an empty display and this log:

Orig dimensions: 595 842
New dimensions: 1 1


For comparison, RotateBrush does work still...

I don't know if this is only Polybios related bug or generally for all kinds of vector brushes?
User avatar
airsoftsoftwair
Posts: 5433
Joined: Fri Feb 12, 2010 2:33 pm
Location: Germany
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by airsoftsoftwair »

No, SVG images work. Only Polybios seems to be affected. Will be fixed, thanks for reporting!
User avatar
jPV
Posts: 603
Joined: Sat Mar 26, 2016 10:44 am
Location: RNO
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by jPV »

I also have a problem when I tried to add PDF support to RNOComics, it seems I get crashes with DisplayBrush() if I use Width/Height options in the table... and give values different to original dimensions. For some reason I haven't been able to reproduce it in a simple example... so I'm not sure if I'm doing something else in the code that affects to the result.

But if you have any hunch what it could be, or is it related to the ScaleBrush issue, please let me know. If not, I'll continue random testing :)

Here are some logs how it crashes: link
User avatar
airsoftsoftwair
Posts: 5433
Joined: Fri Feb 12, 2010 2:33 pm
Location: Germany
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by airsoftsoftwair »

Very likely to be related to the very same issue but it would still be good if you could provide a reproducable script. Maybe try another platform? MorphOS is very tolerant concerning illegal memory accesses. Maybe try to reproduce it on Linux and Windows which are both much less forgiving than MorphOS :)
PEB
Posts: 567
Joined: Sun Feb 21, 2010 1:28 am

Re: ScaleBrush doesn't work for brushes from PDF

Post by PEB »

This bug still seems to be present---using version 1.0a (28-Jun-18).
User avatar
airsoftsoftwair
Posts: 5433
Joined: Fri Feb 12, 2010 2:33 pm
Location: Germany
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by airsoftsoftwair »

No, I haven't investigated into this at all yet. But the bug is very likely to be in Hollywood, not in Polybios.
User avatar
jPV
Posts: 603
Joined: Sat Mar 26, 2016 10:44 am
Location: RNO
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by jPV »

I can now reproduce my crash issue... it's mostly related brush freeing before getting a new brush for the same ID. AFAIK Hollywood should free the previous brush automatically and you haven't needed to use the FreeBrush() command usually when re-using the ID, but that doesn't seem to happen with Polybios. If you don't free the previous brush manually, there seems to be memory leaking. This alone doesn't seem to make the crash here (on MorphOS), but if I also display the brush with custom dimensions, then it crashes sooner or later.

So, these are my observations now (I've used this PDF when testing):

1) This code will leak memory until all is gone. The memoy isn't free'ed when you quit the program (at least under MorphOS). It also leaks under Windows with a quick test (not sure about freeing though, probably frees?).

Code: Select all

@REQUIRE "polybios"
EscapeQuit(True)
pdf.OpenDocument(1, "Apollo_datasheet.pdf")
PDF_DOCUMENT = pdf.GetObjectType()
numpages = GetAttribute(PDF_DOCUMENT, 1, #PDFATTRPAGES)
Repeat
    For i=1 To numpages
        pdf.GetBrush(1, i, 1)
        DisplayBrush(1, 0, 0)
        ;FreeBrush(1)    ; Enabling this would fix the problem
    Next
Forever

2) Adding image scaling will cause the same example to crash after some rounds (randomish) under MorphOS, but with a quick try under Windows it didn't crash, just consumed memory increasingly.

Code: Select all

@REQUIRE "polybios"
EscapeQuit(True)
pdf.OpenDocument(1, "Apollo_datasheet.pdf")
PDF_DOCUMENT = pdf.GetObjectType()
numpages = GetAttribute(PDF_DOCUMENT, 1, #PDFATTRPAGES)
Repeat
    For i=1 To numpages
        pdf.GetBrush(1, i, 1)
        DisplayBrush(1, 0, 0, {Width=GetAttribute(#DISPLAY,0,#ATTRWIDTH), Height=GetAttribute(#DISPLAY,0,#ATTRHEIGHT)}) ; <-- makes it unstable
        ;FreeBrush(1)    ; Enabling this would fix the problem
    Next
Forever

So, is this a user error or should Hollywood/Polybios handle this itself? I think I've only seen a mention to free the brushes before closing the document, but not before getting a new page...
User avatar
airsoftsoftwair
Posts: 5433
Joined: Fri Feb 12, 2010 2:33 pm
Location: Germany
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by airsoftsoftwair »

I think both issues are fixed now. The original issue of this thread (related to ScaleBrush()) doesn't appear any longer with the latest Hollywood master. It turns out that this commit from earlier this year fixed the ScaleBrush() issue as well:

Code: Select all

- Fix: Converting a vector brush into a BGPic didn't work correctly; it was impossible to transform the BGPic afterwards
The second issue has just been fixed in Polybios. The problem was that pdf.GetBrush() will trash memory if the destination brush exists *and* is a brush that has previously been allocated by pdf.GetBrush(). If that is the case, Polybios will become very unstable because internal lists are trashed and anything can happen. Here is a fixed build for MorphOS: http://www.softwarefailure.de/tmp/polyb ... 011118.lha Please test and report if this solves the issue.

Note that the 1x1 brush issue was fixed in Hollywood itself so it will still be there even with this new build of Polybios.
User avatar
jPV
Posts: 603
Joined: Sat Mar 26, 2016 10:44 am
Location: RNO
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by jPV »

airsoftsoftwair wrote: Thu Nov 01, 2018 5:16 pm The second issue has just been fixed in Polybios. The problem was that pdf.GetBrush() will trash memory if the destination brush exists *and* is a brush that has previously been allocated by pdf.GetBrush(). If that is the case, Polybios will become very unstable because internal lists are trashed and anything can happen. Here is a fixed build for MorphOS: http://www.softwarefailure.de/tmp/polyb ... 011118.lha Please test and report if this solves the issue.
Yes, the crash/memory issue seems to be fixed with this!

So, to make sure, is it still safe to use the older one if I free the brush always before replacing it? Or are you planning to release an official Polybios update soon? I was planning to release RNOComics & RNOPDF soonish, but should I wait or do I release them with the old Polybios?
User avatar
airsoftsoftwair
Posts: 5433
Joined: Fri Feb 12, 2010 2:33 pm
Location: Germany
Contact:

Re: ScaleBrush doesn't work for brushes from PDF

Post by airsoftsoftwair »

jPV wrote: Thu Nov 01, 2018 7:15 pm So, to make sure, is it still safe to use the older one if I free the brush always before replacing it? Or are you planning to release an official Polybios update soon? I was planning to release RNOComics & RNOPDF soonish, but should I wait or do I release them with the old Polybios?
No, calling FreeBrush() is completely safe. The bug will only be triggered if pdf.GetBrush() frees the brush.
Post Reply