Page 1 of 1

ScaleBrush doesn't work for brushes from PDF

Posted: Sun May 20, 2018 10:11 am
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?

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Sun May 20, 2018 11:54 pm
by airsoftsoftwair
No, SVG images work. Only Polybios seems to be affected. Will be fixed, thanks for reporting!

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Mon May 21, 2018 4:51 pm
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

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Wed May 23, 2018 7:57 pm
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 :)

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Wed Jul 11, 2018 11:18 pm
by PEB
This bug still seems to be present---using version 1.0a (28-Jun-18).

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Thu Jul 12, 2018 5:40 pm
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.

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Tue Oct 30, 2018 5:04 pm
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...

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Thu Nov 01, 2018 5:16 pm
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.

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Thu Nov 01, 2018 7:15 pm
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?

Re: ScaleBrush doesn't work for brushes from PDF

Posted: Thu Nov 01, 2018 8:40 pm
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.