<div dir="ltr"><div>Hi Eliot,</div><br><div class="gmail_quote"><div dir="ltr">On Thu, May 24, 2018 at 9:51 PM Eliot Miranda <<a href="mailto:eliot.miranda@gmail.com">eliot.miranda@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr">Hi Fabio,<br><div class="gmail_extra"><br><div class="gmail_quote"></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 23, 2018 at 1:22 PM, Fabio Niephaus <span dir="ltr"><<a href="mailto:lists@fniephaus.com" target="_blank">lists@fniephaus.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br><div dir="ltr">Hi Eliot,<br><br><br><div class="gmail_quote"><div dir="ltr">On Wed, May 23, 2018 at 9:48 PM Eliot Miranda <<a href="mailto:eliot.miranda@gmail.com" target="_blank">eliot.miranda@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr">Hi Fabio,<div><br></div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote"></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 23, 2018 at 12:11 PM, Fabio Niephaus <span dir="ltr"><<a href="mailto:lists@fniephaus.com" target="_blank">lists@fniephaus.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <br><div dir="ltr">Hi all,<div><br></div><div>I noticed that BitBlt's primitiveDisplayString fails relatively often and then found out that it's sometimes called to draw empty strings.</div><div><br></div><div>`(ByteString allInstances select: [:ea | ea isEmpty]) size` revealed that there are 2245 empty strings in my image and I assume some of them must be either in the FrameRateMorph morph or in the Clock morph.<br></div><div><br></div><div>Anyway...looking at BitBlt>>#primDisplayString:from:to:map:xTable:kern:, the fallback code basically does nothing if stopIndex is zero (`startIndex to: stopIndex do: ...`) which makes sense.</div><div><br></div><div>What I think doesn't make sense is that the primitive fails in the first place. Instead of failing if stopIndex is not greater 0, I would suggest to use: `stopIndex < 1 ifTrue: [^ interpreterProxy pop: 6].` which cleans up the stack, leaves the receiver, and returns early.</div><div><br></div><div>Let me know what you think!</div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I don't object.  So you suggest the following?</div><div><br></div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">   </span>((interpreterProxy isArray: xTable)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">       </span> and: [(interpreterProxy isArray: glyphMap)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">       </span> and: [(interpreterProxy slotSizeOf: glyphMap) = 256</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">      </span> and: [(interpreterProxy isBytes: sourceString)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">   </span> and: [startIndex > 0</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">  </span> and: [stopIndex > 0</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">   </span> and: [stopIndex <= (interpreterProxy byteSizeOf: sourceString)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">        </span> and: [(self loadBitBltFrom: bbObj)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">       </span> and: [combinationRule ~= 30 "these two need extra source alpha"</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">        </span> and: [combinationRule ~= 31]]]]]]]]]) ifFalse:</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">           </span>[^interpreterProxy primitiveFail].</div><div><br></div><div>=></div><div><br></div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">  </span>((interpreterProxy isArray: xTable)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">       </span> and: [(interpreterProxy isArray: glyphMap)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">       </span> and: [(interpreterProxy slotSizeOf: glyphMap) = 256</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">      </span> and: [(interpreterProxy isBytes: sourceString)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">   </span> and: [startIndex > 0</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">  </span> <b>and: [stopIndex >= 0 "to avoid failing for empty strings..."</b></div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">     </span> and: [stopIndex <= (interpreterProxy byteSizeOf: sourceString)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">        </span> and: [(self loadBitBltFrom: bbObj)</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">       </span> and: [combinationRule ~= 30 "these two need extra source alpha"</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">        </span> and: [combinationRule ~= 31]]]]]]]]]) ifFalse:</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">           </span>[^interpreterProxy primitiveFail].</div><div><br></div><div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">    </span>stopIndex = 0 ifTrue:</div><div><span class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail-Apple-tab-span" style="white-space:pre-wrap">             </span>[^interpreterProxy pop: 6. "the string is empty; pop args, return rcvr"]. </div></div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div></div><div class="gmail_extra"><br></div>Any other opinions either way?</div></div></blockquote><div><br></div><div>Why not do the stopIndex = 0 check early and move it all the way up? The primitive won't fail if any of the other checks fail which I don't think is bad if you don't want to draw anything (stopIndex = 0). On the other hand, these other checks are not executed at all (e.g. loadBitBltFrom:) and you don't have to change the stopIndex > 0 check. :)</div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Because it feels hack to me to not fail the primitive if one supplied an empty string but an otherwise corrupted bitblt obj.  IMO, the primitive should succeed only if all its input arguments are correct.  Pushing the stopIndex check earlier circumvents these checks.  I'll commit the above and if you feel strongly you can commit an updated version (that documents your preference).</div></div></div></div></blockquote><div><br></div><div>All good and thanks! This is the first time I'm proposing changes to the VMMaker code base, so I'm just learning best practices from you here :)</div><div><br></div><div>Fabio</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Fabio<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="m_-8071220327851190246m_-5319373542170699458m_769832743867728791gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</div></div>
</blockquote></div></div>
</blockquote></div><br><br><div class="m_-8071220327851190246gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</div></div>
</blockquote></div></div>